diff --git a/changelog.d/fix-code-execution-abort-while-starting.fixed.md b/changelog.d/fix-code-execution-abort-while-starting.fixed.md index 046fd5e4e..abbea900a 100644 --- a/changelog.d/fix-code-execution-abort-while-starting.fixed.md +++ b/changelog.d/fix-code-execution-abort-while-starting.fixed.md @@ -1,4 +1,4 @@ - A `code_execution` script whose interpreter is still starting is now stopped by `PyRunner.abort()` and `dispose()`, as a running one is, and so is one aborted on the tick `exec` is called. The exec settles at once, and the script never runs. Before, an abort in that window did nothing, though `busy` was already true: when a script's turn ended or was reset to run again in that window, the script ran to its end or its time limit, calling tools as it went, and each `code_execution` call of the agent's later turns was refused as another script already running until then. A dispose there, as a background script's cancel does, waited out the interpreter's 15 s start timeout and then reported that the runtime had failed to start. The interpreter starts at an agent's first script, after it has been idle, and after a script is aborted, killed at its time limit or crashes. - A `code_execution` interpreter that stops reading its input, as one does after a script closes its standard input (`os.close(0)`), is now ended at the first message that can't reach it, and the script waiting on it settles at once, saying the runtime stopped reading its input. That message's failed write was thrown in the host as an uncaught exception (`write EPIPE`), which ended the host's process. After a script's `os.close(0)`, the next script's message is sometimes written without an error and never runs, and that script settles this way only at its time limit. An interpreter that died before its exit was handled still settles with its own exit code. -- A `code_execution` script stopped at its time limit now reads the note "script stopped: it reached its … time limit" on a line of its own when the text before it doesn't end with a newline, as the runner's own texts don't, such as "python runtime exited unexpectedly (…)". The note used to run on at the end of that text's line. +- A `code_execution` script stopped at its time limit now reads the note "script stopped: it reached its … time limit" on a line of its own when the text before it doesn't end with a newline, as the runner's own texts don't, such as an interpreter's exit. The note used to run on at the end of that text's line. - An abort or a dispose on the tick `exec` is called, when the interpreter couldn't be spawned, no longer signals another process. The runner killed the failed spawn's child, which has no pid, in the tick before Node's `error` for it, and Node then signals whatever pid the child's handle held: in a fresh Node 20 process that was 0, so `SIGKILL` went to the host's own process group. diff --git a/changelog.d/fix-presentation-script-errors.fixed.md b/changelog.d/fix-presentation-script-errors.fixed.md new file mode 100644 index 000000000..526e7986a --- /dev/null +++ b/changelog.d/fix-presentation-script-errors.fixed.md @@ -0,0 +1,5 @@ +- `set_tool_visibility` and `set_tool_description` refusals now lead with "Nothing was changed", lose the "Error: " prefix, and say what helps. Arguments of the wrong shape are named together, each by its key, with the call that works. A tool name that isn't available is quoted, cut past 60 characters, and gets the closest names, quoted whole, and the catalogue's index to copy from. A description over 32,768 characters, or hiding one of the three tools that can't be hidden, is refused as the agent's own input, where before it read as a problem with the file; a description's length is now counted in code points rather than UTF-16 units, so a character outside the Basic Multilingual Plane, such as 😀, counts once rather than twice. A stale `.lock`, a presentation file that is a symbolic link, an edit that would take the file past 256 KiB (with the bytes over), and a file changed during the edit each say what happened and whether calling again helps. An edit whose lock can't be removed after it's done now succeeds where it failed, and says that its lock was left and that edits will be refused until it's removed; a refusal that leaves its lock says so too. A lock that something else removed first is not reported as left. +- A presentation or description-defaults file the host can't use is reported with its problems all at once (up to ten, then how many more), each by its entry, both by the editing tools and in the catalogue's diagnostics. A name a problem quotes is cut past 60 characters and a value past 40, counted as code points, and the key the catalogue reports as not currently available is cut as a name is. The diagnostics also say what applies meanwhile and who can fix the file, each on one line of the catalogue. A presentation file that is a symbolic link to nothing is reported as such, where it read as no file. `parsePresentation`'s error names the document's problems the same way, where before it named the first, in other words. +- A `code_execution` script stopped at its time limit says its tool calls keep their effects, and offers `time_limit_ms` only when a longer limit is allowed. At the deployment's ceiling, which is the default unless `maxScriptTimeoutMs` raises it, it says that is the longest a script runs and to split the work, or to run it in the background when this agent could start a background script as the script starts (it runs them, and a slot is free) and one may run longer (a background script: that starting it again runs it from the top). Before, it said only that the script had stopped. A script whose interpreter exits under it, can't start, or exits as it starts says what ran and whether running it again can help; a missing `python3` says `code_execution` can't run there until the host installs one, a non-executable one until the host makes it executable, and a failure creating or writing the runtime's files says so, with its path. A second script while one runs says they run one at a time. +- Inside a script, a tool function called with keyword arguments, such as `await workspace__read(path="a.txt")`, is refused with the dict to write, `await workspace__read({'path': })`, where before Python reported an unexpected keyword argument. A `str` key in that dict is shown whole up to 60 characters, and a longer one, any other key and every value by its type; a non-dict argument is named the same way. +- **Hosts driving `PyRunner`:** `exec()`'s options take `ceilingMs`, the longest limit the deployment allows that kind of script; without it, a stopped script isn't offered a longer limit. A foreground exec's `backgroundCeilingMs` is the longest a background script may run, which a script stopped at its ceiling is offered when it's longer; leave it out where the caller can't start one. diff --git a/src/closest-names.ts b/src/closest-names.ts new file mode 100644 index 000000000..36f75c016 --- /dev/null +++ b/src/closest-names.ts @@ -0,0 +1,94 @@ +/** + * The names someone most likely meant, when the one they gave matches none: + * for a refusal's "did you mean", such as a tool name an agent got wrong. + * + * Case is ignored. A candidate that the name matches as a whole `--` segment + * ("ls" for "workspace--ls"), then one that starts with it or that it starts + * with, then one that contains it or that it contains, ranks ahead of one + * that is only a few edits away, however many edits those matches take. The + * prefix and substring matches need three characters on the shorter side, + * since a letter or two is in nearly every name. A candidate that matches in + * none of those ways counts only within the edits noiseLimit allows, and past + * that it's noise. So a name that is part of no candidate, holds none and is + * close to none gets nothing back, and a name in a crowded namespace doesn't + * get its neighbours (`workspace--write` for `workspace--read`). + */ +export function closestNames(name: string, candidates: readonly string[], max = 2): string[] { + const wanted = name.toLowerCase(); + const ranked: { candidate: string; tier: number; distance: number }[] = []; + for (const candidate of new Set(candidates)) { + const other = candidate.toLowerCase(); + const shorter = Math.min(wanted.length, other.length); + const tier = other === wanted ? 0 + : wanted && other.split('--').includes(wanted) ? 1 + : shorter >= 3 && (other.startsWith(wanted) || wanted.startsWith(other)) ? 2 + : shorter >= 3 && (other.includes(wanted) || wanted.includes(other)) ? 3 + : 4; + // In tiers 0 to 3 one name holds the other, so the length difference is + // already their distance, and a long name skips the edit table. + const limit = tier === 4 ? noiseLimit(wanted, other) : Infinity; + const distance = tier === 4 ? editDistance(wanted, other, limit) : Math.abs(wanted.length - other.length); + if (distance > limit) continue; + ranked.push({ candidate, tier, distance }); + } + // A stable sort: candidates that tie keep the order they were given in. + ranked.sort((a, b) => a.tier - b.tier || a.distance - b.distance); + return ranked.slice(0, Math.max(0, max)).map((entry) => entry.candidate); +} + +/** + * Whether `name` looks like a slip for `candidate`, ignoring case: the same; + * a few edits apart, where a transposition counts as one edit; or, at three + * characters or more, a part of it, which is a shortened attempt at it + * (`mode` for `deliveryMode`, `interval` for `refillIntervalMs`). A part + * counts only that way round: a name that holds the candidate, and is more + * edits from it than a slip allows, is another name, usually an extra that + * explains it (`channel_note` beside `channel`, where `channels` is a slip). + * Beyond these, two names are different names rather than one mistyped. + */ +export function isNearMiss(name: string, candidate: string): boolean { + const a = name.toLowerCase(), b = candidate.toLowerCase(); + if (a.length >= 3 && b.includes(a)) return true; + const limit = noiseLimit(a, b); + return editDistance(a, b, limit) <= limit; +} + +/** + * The most edits that still read as a slip: a quarter of what follows the + * prefix the two names share, and at least one. A shared prefix is mostly a + * namespace (`workspace--`, `mcpl--discord--`) and says nothing about the + * rest, so `workspace--read` and `workspace--write` are four edits apart in + * a four-letter tool name, not in a fifteen-letter one. + */ +function noiseLimit(a: string, b: string): number { + let shared = 0; + while (shared < a.length && shared < b.length && a[shared] === b[shared]) shared++; + return Math.max(1, Math.floor((Math.max(a.length, b.length) - shared) / 4)); +} + +/** + * Edits between two strings: insertions, deletions, substitutions and + * swaps of neighbours (optimal string alignment). Past `limit` the exact + * count doesn't matter, so it answers `limit + 1` as soon as it is sure. + */ +function editDistance(a: string, b: string, limit: number): number { + if (Math.abs(a.length - b.length) > limit) return limit + 1; + let before: number[] = []; + let previous = Array.from({ length: b.length + 1 }, (_, j) => j); + for (let i = 1; i <= a.length; i++) { + const current = [i]; + let best = i; + for (let j = 1; j <= b.length; j++) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1; + let value = Math.min(previous[j] + 1, current[j - 1] + 1, previous[j - 1] + cost); + if (i > 1 && j > 1 && a[i - 1] === b[j - 2] && a[i - 2] === b[j - 1]) value = Math.min(value, before[j - 2] + 1); + current.push(value); + best = Math.min(best, value); + } + // No row's smallest entry is below the one before it, so none later can come back under the limit. + if (best > limit) return limit + 1; + before = previous; + previous = current; + } + return previous[b.length]; +} diff --git a/src/code-execution/py-runner.ts b/src/code-execution/py-runner.ts index 3b37f553a..a390ec2c9 100644 --- a/src/code-execution/py-runner.ts +++ b/src/code-execution/py-runner.ts @@ -19,6 +19,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { createInterface, type Interface } from 'node:readline'; +import { boundedErrorMessage } from '../error-message.js'; import { PYTHON_RUNTIME_SOURCE } from './runtime-py.js'; export interface InjectedTool { @@ -91,6 +92,56 @@ export function formatLimit(ms: number): string { return `${Math.round(ms / 360_000) / 10} h`; } +/** + * What a script stopped at its time limit is told after why: that the tool + * calls it made keep their effects, and whether a longer limit can help, + * which only the deployment's ceiling for its kind of script can say. A + * foreground script at its ceiling is offered background mode too, when its + * caller passes `backgroundCeilingMs` (the framework does for an agent that + * could start a background script as the script starts) and a background + * script may run longer. + */ +function limitRemedy(limitMs: number, ceilingMs: number | undefined, background: boolean, backgroundCeilingMs?: number): string { + const effects = 'Tool calls it had already made keep their effects, so check them before running it again.'; + if (ceilingMs === undefined) return effects; + if (limitMs < ceilingMs) return `${effects} To give it longer, ${background ? 'start it with' : 'pass'} time_limit_ms up to ${ceilingMs} (${formatLimit(ceilingMs)}).`; + if (background) return `${effects} ${formatLimit(limitMs)} is the longest a background script runs here; starting it again runs it from the top.`; + const inBackground = backgroundCeilingMs !== undefined && backgroundCeilingMs > limitMs + ? `, or run it with background: true, where a script can run up to ${formatLimit(backgroundCeilingMs)}` : ''; + return `${effects} ${formatLimit(limitMs)} is the longest a script can run here, so split the work across shorter scripts${inBackground}.`; +} + +/** + * Why a script didn't run when its interpreter didn't start. A spawn that + * found nothing it could run (ENOENT, EACCES) fails the same way until the + * host changes, so that one says code_execution can't run here rather than + * to try again. A failure before the spawn, while creating or writing the + * runtime's files, is about those files rather than the interpreter, and + * keeps Node's own text, which names the path for an open or a mkdtemp. + */ +function startFailure(pythonPath: string, err: unknown): string { + const { code, syscall } = (err ?? {}) as { code?: unknown; syscall?: unknown }; + const spawning = typeof syscall === 'string' && syscall.startsWith('spawn'); + if (spawning && (code === 'ENOENT' || code === 'EACCES')) { + const until = code === 'ENOENT' ? 'installs it or points this host at one' : 'makes it executable or points this host at another'; + return `script didn't run: this host can't start its Python interpreter, ${pythonPath} (${code}), so code_execution can't run scripts here until whoever runs this host ${until}. Calling it again before then fails the same way.`; + } + if (typeof syscall === 'string' && !spawning) { + return `script didn't run: this host couldn't create or write the files its Python runtime starts from (${boundedErrorMessage(err)}), so none of the script ran. Run it again; if it fails the same way, ask whoever runs this host.`; + } + const why = typeof code === 'string' ? `couldn't be started (${code})` : boundedErrorMessage(err); + return `script didn't run: the Python interpreter, ${pythonPath}, ${why}, so none of the script ran. Run it again; if it fails the same way, ask whoever runs this host.`; +} + +/** Why a script stopped when its interpreter exited under it, and what running it again does. */ +function interpreterExit(exitCode: number | null, signal: NodeJS.Signals | null, background: boolean): string { + const how = signal ? `was ended by ${signal}` : `exited with code ${exitCode}`; + return `script stopped: the Python interpreter ${how} while it ran, so the rest of the script didn't run` + + `${background ? '' : ', and variables from earlier scripts are gone'}. Tool calls it had already made keep their effects, ` + + 'so check them before you run it again in a new interpreter. If it stops the same way, check whether the script itself ' + + `ends the process, for example ${signal ? 'by using too much memory' : 'with os._exit()'}; if not, ask whoever runs this host.`; +} + interface PendingExec { id: string; resolve: (result: ExecResult) => void; @@ -101,6 +152,12 @@ interface PendingExec { settled: boolean; /** Set once the deadline fired: the result then says the script ran out of time. */ deadlineMs: number | null; + /** The longest limit this exec could have had (exec's opts): whether a longer one would help. */ + ceilingMs: number | undefined; + /** For a foreground exec, the longest a background script may run (exec's opts): whether background mode would help. */ + backgroundCeilingMs: number | undefined; + /** A background exec has its interpreter to itself, so no earlier script's state is lost with it. */ + background: boolean; /** Set once the script went to the interpreter. Before that, the exec is * waiting for the interpreter to start, and an interpreter that exits then * failed to start (see exec). */ @@ -152,12 +209,18 @@ export class PyRunner { * * `opts.deadlineMs` replaces the runner's scriptTimeoutMs for this exec (a * per-call time limit); a background exec uses its `lifetimeMs` instead. + * `opts.ceilingMs` is the longest limit the deployment allows this kind of + * script: a script stopped at its limit is offered a longer one only below + * it. Without it, the stop doesn't say whether a longer limit would help. + * `opts.backgroundCeilingMs`, for a foreground exec, is the longest a + * background script may run: one stopped at its ceiling is offered + * background mode when that is longer. */ async exec( code: string, tools: InjectedTool[], background?: BackgroundExecOptions, - opts?: { deadlineMs?: number }, + opts?: { deadlineMs?: number; ceilingMs?: number; backgroundCeilingMs?: number }, ): Promise { if (this.disposed) { return { stdout: '', stderr: 'code_execution runner disposed', returnCode: 1, aborted: true }; @@ -165,7 +228,8 @@ export class PyRunner { if (this.busy) { return { stdout: '', - stderr: 'RuntimeError: another code_execution script is already running for this agent', + stderr: "script didn't run: another code_execution script of yours is already running, and they run one at a time. " + + 'Run this one again once that one has finished, or combine the two into one script.', returnCode: 1, aborted: true, }; @@ -186,6 +250,9 @@ export class PyRunner { killTimer: null, settled: false, deadlineMs: null, + ceilingMs: opts?.ceilingMs, + backgroundCeilingMs: opts?.backgroundCeilingMs, + background: background !== undefined, sent: false, }; this.pending = pending; @@ -195,10 +262,9 @@ export class PyRunner { // Settled already when an abort or dispose reclaimed the interpreter // as it started; another exec may have claimed the runner since. if (!pending.settled) { - const message = err instanceof Error ? err.message : String(err); this.settlePending({ stdout: '', - stderr: `Failed to start python runtime (${this.pythonPath}): ${message}`, + stderr: startFailure(this.pythonPath, err), returnCode: 1, aborted: true, }); @@ -230,8 +296,9 @@ export class PyRunner { this.settlePending({ stdout: '', stderr: - `script stopped: it reached its ${formatLimit(deadlineMs)} time limit and did not respond, ` + - 'so it was killed and the interpreter restarted (variables from earlier scripts are gone)', + `script stopped: it reached its ${formatLimit(deadlineMs)} time limit and did not respond, so it was killed` + + (pending.background ? '. ' : ' and the interpreter restarted (variables from earlier scripts are gone). ') + + limitRemedy(deadlineMs, pending.ceilingMs, pending.background, pending.backgroundCeilingMs), returnCode: 1, aborted: true, }); @@ -349,7 +416,7 @@ export class PyRunner { ? 'python runtime stopped reading its input, so it was killed and the interpreter restarted ' + '(variables from earlier scripts are gone). A script that closes its standard input ' + '(file descriptor 0) does this.' - : `python runtime exited unexpectedly (code=${exitCode}, signal=${signal ?? 'none'})`, + : interpreterExit(exitCode, signal, this.pending.background), returnCode: 1, aborted: true, }); @@ -369,7 +436,7 @@ export class PyRunner { }; const onExit = () => { cleanup(); - reject(new Error('python runtime exited before becoming ready')); + reject(new Error('exited as it started')); }; const onError = (err: Error) => { cleanup(); @@ -377,7 +444,7 @@ export class PyRunner { }; const timeout = setTimeout(() => { cleanup(); - reject(new Error('python runtime did not become ready within 15s')); + reject(new Error("didn't start within 15 seconds")); }, 15_000); const cleanup = () => { clearTimeout(timeout); @@ -473,7 +540,8 @@ export class PyRunner { // Only a script that ended unsuccessfully was stopped: one that caught the cancellation // and finished its work returns 0, and must not read as stopped. if (pending.deadlineMs !== null && result.returnCode !== 0) { - const note = `script stopped: it reached its ${formatLimit(pending.deadlineMs)} time limit\n`; + const note = `script stopped: it reached its ${formatLimit(pending.deadlineMs)} time limit. ` + + `${limitRemedy(pending.deadlineMs, pending.ceilingMs, pending.background, pending.backgroundCeilingMs)}\n`; // On a line of its own: the runner's own texts, such as an interpreter's exit, end without a newline. const withNote = (text: string) => (text === '' || text.endsWith('\n') ? text : `${text}\n`) + note; // A background script reports its output tail, so the note goes there too. @@ -539,8 +607,8 @@ export class PyRunner { // An interpreter killed as it starts never says it's ready: fail its // readiness now, so an exec waiting on it returns at once rather than at // the 15 s timeout. Here rather than in teardownChild, which an - // interpreter's own exit also runs, so that one still reads "exited - // before becoming ready". + // interpreter's own exit also runs, so that one still reads "exited as + // it started". this.readyFailer?.(new Error(`python runtime reclaimed (${reason}) before it was ready`)); this.teardownChild(); this.clearIdleTimer(); diff --git a/src/code-execution/runtime-py.ts b/src/code-execution/runtime-py.ts index e1304f557..9e7443934 100644 --- a/src/code-execution/runtime-py.ts +++ b/src/code-execution/runtime-py.ts @@ -259,15 +259,61 @@ def _resolve(fut, value): pass +def _brief(value): + # A value as a refusal shows it: by its type, such as , which fails + # loudly if copied rather than sending something else. The agent has its + # values in the call it just wrote. No value's own code runs here: its + # __repr__ isn't called, and its type is named through type's own + # descriptor, so a metaclass's __name__ doesn't run either. + return "<" + type.__dict__["__name__"].__get__(type(value)) + ">" + + def _make_tool_fn(tool_name, py_name): - async def tool_fn(args=None): + async def tool_fn(*positional, **kwargs): global _next_tool_call_id + if len(positional) > 1: + raise TypeError( + py_name + " wasn't called: a tool function takes one dict of " + "arguments, such as await " + py_name + "({...}), and this call " + "gave it " + str(len(positional)) + " positional arguments" + ) + # args= on its own is the dict, as it has always been. Beside other + # keywords it's one of them, since a tool such as utils has a field + # of that name, so the suggested dict keeps it. + if positional: + args = positional[0] + elif list(kwargs) == ["args"]: + args = kwargs.pop("args") + else: + args = None + if kwargs: + if args is not None and not isinstance(args, dict): + raise TypeError( + py_name + " wasn't called: a tool function takes one dict of " + "arguments, not keyword arguments, and this call also gave it " + + _brief(args) + ", so put every argument in one dict: await " + + py_name + "({...})" + ) + # The likely intent is plain: the same arguments in one dict. + meant = dict(args) if isinstance(args, dict) else {} + meant.update(kwargs) + # A key is what the agent has to write, so a str key is shown + # whole up to 60 characters, as names are elsewhere; a longer one, + # or any other key, is shown as a value is. + shown = ", ".join((repr(k) if type(k) is str and len(k) <= 60 else _brief(k)) + ": " + _brief(v) + for k, v in meant.items()) + raise TypeError( + py_name + " wasn't called: a tool function takes one dict of " + "arguments, not keyword arguments, so write await " + + py_name + "({" + shown + "})" + ) if args is None: args = {} if not isinstance(args, dict): raise TypeError( - "tool functions take a single dict of arguments, e.g. await " - + py_name + "({...})" + py_name + " wasn't called: a tool function takes one dict of " + "arguments, such as await " + py_name + "({...}), and this call " + "gave it " + _brief(args) ) _next_tool_call_id += 1 call_id = "t" + str(_next_tool_call_id) diff --git a/src/error-message.ts b/src/error-message.ts new file mode 100644 index 000000000..6f6db4d1a --- /dev/null +++ b/src/error-message.ts @@ -0,0 +1,48 @@ +/** + * What a thrown value says: an Error's message, or any other value as a + * string, without the "Error: " that `String(error)` puts in front. + * + * It never throws. Every inspection and coercion of the value is inside the + * try, so a revoked Proxy (whose instanceof throws), a hostile getter, an + * Error whose message isn't a string, or a value with no string form reads + * as text or as unreadable. It doesn't bound the text: a message the module + * wrote for the agent often ends with its remedy, which a bound would cut. A + * caller that writes it to a log or a server's answer bounds it there, with + * boundedErrorMessage. + */ +export function errorMessage(error: unknown): string { + try { + return String(error instanceof Error ? error.message : error); + } catch { + return '(a value that could not be read)'; + } +} + +/** + * The most of a thrown value's text that reaches a log line, a server's error + * answer or an agent's frame around an error nobody expected. + */ +export const THROWN_TEXT_LIMIT = 500; + +/** + * errorMessage, cut to `limit` characters with the whole length said, for a + * value whose text nobody wrote for its reader: a handler's throw sent on as + * an answer, or an error a module didn't expect quoted inside its own frame, + * since a provider or handler error can carry a whole request body. + * + * Characters are code points, so a cut never leaves half of a surrogate pair + * at the end. The text is walked rather than spread into an array, which for + * a whole request body would hold every code point at once. + */ +export function boundedErrorMessage(error: unknown, limit = THROWN_TEXT_LIMIT): string { + const text = errorMessage(error); + // A code point takes one or two UTF-16 units, so text this short is within the limit. + if (text.length <= limit) return text; + let points = 0; + let cut = 0; + for (const point of text) { + if (points < limit) cut += point.length; + points++; + } + return points <= limit ? text : `${text.slice(0, cut)}… (${points} characters)`; +} diff --git a/src/framework.ts b/src/framework.ts index 7fe1bdda8..4d01ed174 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -15217,7 +15217,12 @@ export class AgentFramework { if (ownsOrigin) this.foregroundScriptOrigins.set(agentName, origin); let exec: import('./code-execution/py-runner.js').ExecResult; try { - exec = await runner.exec(input.code, injected, undefined, timeLimitMs !== undefined ? { deadlineMs: timeLimitMs } : undefined); + // A script stopped at its ceiling is offered background mode only when + // this agent could start a background script as this one starts: it + // runs them, and a slot is free under the cap. + const backgroundCeilingMs = this.runsBackgroundScripts(agentName) + && this.runningBackgroundScripts(agentName) < this.maxBackgroundScripts() ? limits.backgroundMaxMs : undefined; + exec = await runner.exec(input.code, injected, undefined, { deadlineMs: timeLimitMs, ceilingMs, backgroundCeilingMs }); } finally { if (this.scriptDeferredEndTurn.get(agentName) === script) this.scriptDeferredEndTurn.delete(agentName); if (ownsOrigin && this.foregroundScriptOrigins.get(agentName) === origin) { @@ -15243,6 +15248,21 @@ export class AgentFramework { // Background (daemon) scripts — model-authored watchers that outlive the turn // --------------------------------------------------------------------------- + /** Background scripts are the primary agent's, or any agent's when no primary agent is named. */ + private runsBackgroundScripts(agentName: string): boolean { + return !this.primaryAgentName || agentName === this.primaryAgentName; + } + + /** How many background scripts an agent may have running at once. */ + private maxBackgroundScripts(): number { + return this.codeExecutionConfig?.maxBackgroundScripts ?? 3; + } + + /** How many of this agent's background scripts are running now. */ + private runningBackgroundScripts(agentName: string): number { + return [...this.backgroundScripts.values()].filter((s) => s.agentName === agentName && s.status === 'running').length; + } + /** * Start a background script: dedicated interpreter, output journaled to a * workspace file, wake_agent() available in-script. The tool result @@ -15261,7 +15281,7 @@ export class AgentFramework { // v1: primary-agent-only. A conversation fork's or ephemeral's daemon // would outlive its owner and its wake would land in the primary // conversation anyway — refuse loudly instead of surprising anyone. - if (this.primaryAgentName && agentName !== this.primaryAgentName) { + if (!this.runsBackgroundScripts(agentName)) { return { success: false, error: 'background scripts are available to the primary agent only', @@ -15269,10 +15289,8 @@ export class AgentFramework { }; } const cfg = this.codeExecutionConfig; - const maxScripts = cfg?.maxBackgroundScripts ?? 3; - const runningCount = [...this.backgroundScripts.values()] - .filter((s) => s.agentName === agentName && s.status === 'running').length; - if (runningCount >= maxScripts) { + const maxScripts = this.maxBackgroundScripts(); + if (this.runningBackgroundScripts(agentName) >= maxScripts) { return { success: false, error: `background script limit reached (${maxScripts} running). ` + @@ -15332,7 +15350,7 @@ export class AgentFramework { logPath: logAbsPath, lifetimeMs, onWake: (line, payload) => this.handleScriptWake(record, line, payload), - }) + }, { ceilingMs: scriptTimeLimits(cfg ?? undefined).backgroundMaxMs }) .then((exec) => this.settleBackgroundScript(record, exec)) .catch((err) => { // exec never rejects by contract; this is the belt-and-suspenders. diff --git a/src/tool-presentation.ts b/src/tool-presentation.ts index 8fd9eef42..9e45111c8 100644 --- a/src/tool-presentation.ts +++ b/src/tool-presentation.ts @@ -1,5 +1,7 @@ import { createHash, randomUUID } from 'node:crypto'; import { fchmodSync, closeSync, fstatSync, lstatSync, openSync, readSync, renameSync, unlinkSync, writeFileSync } from 'node:fs'; +import { closestNames } from './closest-names.js'; +import { boundedErrorMessage, errorMessage } from './error-message.js'; import type { ToolDefinition, ToolResult } from './types/events.js'; export interface ToolPresentationConfig { @@ -22,23 +24,138 @@ export interface PresentationSnapshot { entries: { name: string; source: string; visible: boolean; originalDescription: string; description: string; descriptionSource?: string; inputSchema: ToolDefinition['inputSchema'] }[]; } const MAX_BYTES = 256 * 1024; +const MAX_DESCRIPTION_CHARS = 32768; const protectedNames = new Set(['set_tool_visibility', 'set_tool_description', 'workspace--read']); export const isPresentationTool = (name: string): boolean => name === 'set_tool_visibility' || name === 'set_tool_description'; const hash = (value: string): string => createHash('sha256').update(value).digest('hex'); -export function parsePresentation(raw: string): PresentationDocument { - const doc = JSON.parse(raw); - if (!doc || doc.version !== 1 || !doc.tools || typeof doc.tools !== 'object' || Array.isArray(doc.tools) - || Object.keys(doc).some(k => !['version', 'tools'].includes(k))) throw new Error('Expected {"version":1,"tools":{...}}'); - for (const [name, value] of Object.entries(doc.tools)) { - if (!name || !value || typeof value !== 'object' || Array.isArray(value)) throw new Error(`Invalid override: ${name}`); +/** + * A presentation or defaults file this host can't use. Each problem is a + * clause about the file, such as `the entry for "x" has "visible": "no", + * which must be true or false`; whoever reports it names the file and says + * what follows from it. + */ +class PresentationFileError extends Error { + constructor(problems: string[]) { + const more = problems.length - 10; + super((more > 0 ? [...problems.slice(0, 10), `${more} more problem${more === 1 ? '' : 's'}`] : problems).join('; ')); + } +} + +/** An edit refused, in words for the agent: its message is the whole result. */ +class PresentationRefusal extends Error {} + +const count = (n: number): string => n.toLocaleString('en-US'); + +/** A text's length in code points, so a character outside the Basic Multilingual Plane, such as 😀, counts once where UTF-16 counts it twice. */ +const characters = (text: string): number => [...text].length; + +/** A text's first `n` characters, never splitting a code point. */ +const firstCharacters = (text: string, n: number): string => [...text].slice(0, n).join(''); + +/** A name as a message quotes it: whole up to 60 characters, and past that cut, with its length, since a file can hold a key of any size. */ +function quoted(name: string): string { + return characters(name) <= 60 ? JSON.stringify(name) : `${JSON.stringify(firstCharacters(name, 57))}... (${count(characters(name))} characters)`; +} + +/** A file's problem as a message quotes it: its own clauses whole, and anything else bounded, since nobody wrote that text for its reader. */ +const problemText = (error: unknown): string => error instanceof PresentationFileError ? error.message : boundedErrorMessage(error); + +/** A value as a message quotes it: short JSON for a scalar, the kind of anything bigger. */ +function shown(value: unknown): string { + if (Array.isArray(value)) return 'an array'; + if (value !== null && typeof value === 'object') return 'an object'; + const json = JSON.stringify(value) ?? String(value); + return characters(json) > 40 ? `${firstCharacters(json, 37)}...` : json; +} + +/** Names quoted and joined: "a", "a" and "b", "a", "b" and "c"; past ten, the first ten and how many more, since a file can hold any number of keys. Each is quoted by `quote`: quoted() by default, which cuts a file's long keys, or JSON.stringify for names the agent should copy whole. */ +function quotedList(names: string[], conjunction: 'and' | 'or' = 'and', quote: (name: string) => string = quoted): string { + const each = names.slice(0, 10).map((name) => quote(name)); + if (names.length > 10) return `${each.join(', ')} ${conjunction} ${count(names.length - 10)} more`; + return each.length > 1 ? `${each.slice(0, -1).join(', ')} ${conjunction} ${each[each.length - 1]}` : each.join(''); +} + +/** + * Every problem that keeps a document from being used, each by where it is; + * none for a usable one. A defaults file (`descriptionsOnly`) may set + * descriptions only, since visibility is the resident's to choose. + */ +function documentProblems(doc: unknown, descriptionsOnly: boolean): string[] { + if (!doc || typeof doc !== 'object' || Array.isArray(doc)) return [`its top level is ${shown(doc)}, not an object like {"version":1,"tools":{}}`]; + const top = doc as Record; + const problems: string[] = []; + if (top.version !== 1) problems.push(`"version" must be 1, and ${Object.hasOwn(top, 'version') ? `it is ${shown(top.version)}` : "it's missing"}`); + const extra = Object.keys(top).filter((key) => key !== 'version' && key !== 'tools'); + if (extra.length) problems.push(`only "version" and "tools" belong at the top level, not ${quotedList(extra)}`); + const tools = top.tools; + if (!tools || typeof tools !== 'object' || Array.isArray(tools)) { + problems.push(`"tools" must be an object of tool names, and ${Object.hasOwn(top, 'tools') ? `it is ${shown(tools)}` : "it's missing"}`); + return problems; + } + const setsVisibility: string[] = []; + for (const [name, value] of Object.entries(tools)) { + if (!name) { problems.push('"tools" has an entry with an empty name'); continue; } + const entry = `the entry for ${quoted(name)}`; + if (!value || typeof value !== 'object' || Array.isArray(value)) { problems.push(`${entry} is ${shown(value)}, not an object such as {"visible":false}`); continue; } const item = value as Record; - if (Object.keys(item).some(k => !['description', 'visible'].includes(k)) - || ('description' in item && (typeof item.description !== 'string' || item.description.length > 32768)) - || ('visible' in item && typeof item.visible !== 'boolean')) throw new Error(`Invalid description/visibility for ${name}`); - if (protectedNames.has(name) && item.visible === false) throw new Error(`Cannot hide recovery tool ${name}`); + const unknown = Object.keys(item).filter((key) => key !== 'description' && key !== 'visible'); + if (unknown.length) problems.push(`${entry} has ${quotedList(unknown)}, and an entry may have only ${descriptionsOnly ? '"description"' : '"description" and "visible"'}`); + if (Object.hasOwn(item, 'description')) { + if (typeof item.description !== 'string') problems.push(`${entry} has a "description" that is ${shown(item.description)}, not a string`); + else if (characters(item.description) > MAX_DESCRIPTION_CHARS) problems.push(`${entry} has a "description" of ${count(characters(item.description))} characters, over the ${count(MAX_DESCRIPTION_CHARS)} allowed`); + } + if (!Object.hasOwn(item, 'visible')) continue; + if (descriptionsOnly) setsVisibility.push(name); + else if (typeof item.visible !== 'boolean') problems.push(`${entry} has "visible": ${shown(item.visible)}, which must be true or false`); + else if (!item.visible && protectedNames.has(name)) problems.push(`${entry} hides it, which isn't allowed, since workspace--read, set_tool_visibility and set_tool_description are how the catalogue is read and how these settings are undone`); } - return doc; + if (setsVisibility.length) problems.push(`it sets "visible" for ${quotedList(setsVisibility)}, and a defaults file may set only descriptions, since visibility is yours to choose`); + return problems; +} + +function parseDocument(raw: string, descriptionsOnly: boolean): PresentationDocument { + let doc: unknown; + try { doc = JSON.parse(raw); } + catch (error) { throw new PresentationFileError([`it isn't valid JSON: ${errorMessage(error)}`]); } + const problems = documentProblems(doc, descriptionsOnly); + if (problems.length) throw new PresentationFileError(problems); + return doc as PresentationDocument; +} + +/** A resident's presentation document, or an error naming every problem that keeps it from being used. */ +export function parsePresentation(raw: string): PresentationDocument { + return parseDocument(raw, false); +} + +/** + * What an agent is told when an edit fails. Every throw in `edit()` comes + * before its rename, and the cleanup after it never throws, so nothing was + * changed: a refusal says so in its own words, a file this host can't use is + * the host's to repair, and anything else is reported as the error it is. + */ +function editFailure(error: unknown): string { + if (error instanceof PresentationRefusal) return error.message; + if (error instanceof PresentationFileError) return `Nothing was changed: your tool presentation file can't be used (${error.message}), and these tools won't write over it until it's repaired. Ask whoever runs this host to repair it.`; + const code = (error as NodeJS.ErrnoException | null)?.code; + return `Nothing was changed: the edit failed (${typeof code === 'string' ? code : boundedErrorMessage(error)}). Ask whoever runs this host to check your tool presentation file and its directory.`; +} + +/** Whether a path is a symbolic link, so an open that found nothing can tell a dangling link from no file. */ +function isLink(path: string): boolean { + try { return lstatSync(path).isSymbolicLink(); } catch { return false; } +} + +/** Runs one step of an edit's cleanup, which must not throw: it returns what the step threw, if it did. */ +function tidy(step: () => void): { error: unknown } | undefined { + try { step(); return undefined; } catch (error) { return { error }; } +} + +/** What an edit says when its lock couldn't be removed afterwards, since every later edit will meet that lock. */ +function lockLeftBehind(error: unknown): string { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + return `This edit couldn't remove its lock afterwards, the presentation file's name with .lock added${typeof code === 'string' ? ` (${code})` : ''}. ` + + 'Until whoever runs this host removes it, set_tool_visibility and set_tool_description will be refused, saying an edit left its lock behind.'; } export function presentationTools(path: string): ToolDefinition[] { @@ -57,43 +174,50 @@ export class ToolPresentation { private read(path = this.config.path, allowMissing = true): string { let fd: number; try { fd = openSync(path, 'r'); } - catch (error) { if (allowMissing && (error as NodeJS.ErrnoException).code === 'ENOENT') return '{"version":1,"tools":{}}'; throw error; } + catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'ENOENT') { + if (isLink(path)) throw new PresentationFileError(["it's a symbolic link to a file that doesn't exist"]); + if (allowMissing) return '{"version":1,"tools":{}}'; + throw new PresentationFileError(["it doesn't exist"]); + } + throw new PresentationFileError([`it couldn't be opened: ${code ?? boundedErrorMessage(error)}`]); + } try { const stat = fstatSync(fd); - if (!stat.isFile() || stat.size > MAX_BYTES) throw new Error('Presentation must be a regular file under 256 KiB'); + if (!stat.isFile()) throw new PresentationFileError(["it isn't a regular file"]); + if (stat.size > MAX_BYTES) throw new PresentationFileError([`it's ${count(stat.size)} bytes, over the ${count(MAX_BYTES)} (256 KiB) this host reads`]); const data = Buffer.alloc(MAX_BYTES + 1); - let size = 0, count = 0; - while (size < data.length && (count = readSync(fd, data, size, data.length - size, null)) > 0) size += count; - if (size > MAX_BYTES) throw new Error('Presentation exceeds 256 KiB'); + let size = 0, bytesRead = 0; + while (size < data.length && (bytesRead = readSync(fd, data, size, data.length - size, null)) > 0) size += bytesRead; + if (size > MAX_BYTES) throw new PresentationFileError([`it grew past ${count(MAX_BYTES)} bytes (256 KiB) while it was being read`]); return data.subarray(0,size).toString('utf8'); } finally { closeSync(fd); } } resolve(tools: ToolDefinition[], sources: ReadonlyMap = new Map()): PresentationSnapshot { let raw = '', doc: PresentationDocument = {version:1,tools:{}}, diagnostics: string[] = []; try { raw = this.read(); doc = parsePresentation(raw); } - catch (error) { diagnostics.push(`Overrides not applied: ${String(error)}. Default visibility and component descriptions retained.`); } + catch (error) { diagnostics.push(`Your tool presentation file can't be used (${problemText(error)}), so none of its settings apply: every tool is visible and shows its configured default description, or its installed one where none is configured. set_tool_visibility and set_tool_description won't change the file until it's repaired; ask whoever runs this host to repair it.`); } const defaults = new Map(); const configuredSources = new Set(); for (const profile of this.config.defaults ?? []) { if (configuredSources.has(profile.source)) { - diagnostics.push(`Duplicate defaults source ignored: ${profile.source}`); continue; + diagnostics.push(`The description defaults for ${JSON.stringify(profile.source)} are configured twice; only the first is read, and the other is ignored. Ask whoever runs this host to remove the duplicate.`); continue; } configuredSources.add(profile.source); // Absent components contribute neither tools nor missing-file errors. if (!tools.some(t => (sources.get(t.name) ?? 'Unattributed') === profile.source)) continue; try { - const profileDoc = parsePresentation(this.read(profile.path, false)); - if (Object.values(profileDoc.tools).some(item => Object.keys(item).some(k => k !== 'description'))) - throw new Error('Component defaults may contain descriptions only; visibility belongs to the resident'); + const profileDoc = parseDocument(this.read(profile.path, false), true); for (const tool of tools) { if ((sources.get(tool.name) ?? 'Unattributed') !== profile.source) continue; const item = Object.hasOwn(profileDoc.tools, tool.name) ? profileDoc.tools[tool.name] : undefined; if (item?.description !== undefined) defaults.set(tool.name, {description:item.description,path:profile.path}); } - } catch (error) { diagnostics.push(`Component defaults not applied (${profile.source}): ${String(error)}`); } + } catch (error) { diagnostics.push(`The description defaults for ${JSON.stringify(profile.source)} can't be used (${problemText(error)}), so its tools show their installed descriptions, or yours where you set one. Ask whoever runs this host to fix that defaults file.`); } } const names = new Set(tools.map(t => t.name)); - for (const name of Object.keys(doc.tools)) if (!names.has(name)) diagnostics.push(`Not currently available: ${name}`); + for (const name of Object.keys(doc.tools)) if (!names.has(name)) diagnostics.push(`Not currently available: ${quoted(name)}`); const entries = tools.map(tool => { const override = Object.hasOwn(doc.tools, tool.name) ? doc.tools[tool.name] : undefined; // Always retain a usable discovery signpost, even when its wording is overridden. @@ -111,39 +235,98 @@ export class ToolPresentation { edit(tool: string, input: unknown, available: ToolDefinition[]): ToolResult { const lock = this.config.path + '.lock'; let fd: number | undefined, tempFd: number | undefined, temp: string | undefined; + let result: ToolResult, lockLeft: { error: unknown } | undefined; try { - if (!isPresentationTool(tool)) throw new Error('Unknown editing tool'); - const value = input as Record; + if (!isPresentationTool(tool)) throw new Error(`${tool} isn't a tool presentation editing tool`); const field = tool === 'set_tool_visibility' ? 'visible' : 'description'; - if (!value || typeof value !== 'object' || Array.isArray(value) || typeof value.name !== 'string' - || Object.keys(value).some(k=> !['name',field].includes(k)) || !Object.hasOwn(value,field)) throw new Error('Invalid editing arguments'); - if (!available.some(t=>t.name===value.name)) throw new Error('Tool is not currently available to this agent'); - if (field === 'visible' ? typeof value.visible !== 'boolean' : value.description !== null && typeof value.description !== 'string') throw new Error(`Invalid ${field}`); - fd = openSync(lock, 'wx', 0o600); + const name = this.editedName(tool, field, input, available); + const value = input as Record; + try { fd = openSync(lock, 'wx', 0o600); } + catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; + throw new PresentationRefusal("Nothing was changed: an earlier edit of your tool presentation file left its lock behind, or another edit is in progress. Call this again in a few seconds. If it fails the same way, ask whoever runs this host to remove the stale lock, the presentation file's name with .lock added, once nothing is editing the file."); + } let mode = 0o600; - try { const stat = lstatSync(this.config.path); if (stat.isSymbolicLink()) throw new Error('Edit the target file directly; tool editing refuses symlinks'); mode = stat.mode & 0o777; } + try { + const stat = lstatSync(this.config.path); + if (stat.isSymbolicLink()) throw new PresentationRefusal("Nothing was changed: your tool presentation file is a symbolic link, and these tools don't write through links. Ask whoever runs this host to make the change in the file it points to, or to replace the link with that file."); + mode = stat.mode & 0o777; + } catch (error) { if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; } const before = this.read(), doc = parsePresentation(before); - const old = Object.hasOwn(doc.tools,value.name) ? doc.tools[value.name] : {}; + const old = Object.hasOwn(doc.tools,name) ? doc.tools[name] : {}; const updated = {...old}; if (field === 'description' && value.description === null) delete updated.description; else Object.assign(updated,{[field]:value[field]}); - Object.defineProperty(doc.tools,value.name,{value:updated,enumerable:true,writable:true,configurable:true}); + Object.defineProperty(doc.tools,name,{value:updated,enumerable:true,writable:true,configurable:true}); const next = JSON.stringify(doc,null,2)+'\n'; + // editedName has checked what this adds, so this can't fail on the agent's input; + // it keeps a gap in those checks from writing a file the host would then ignore. parsePresentation(next); - if (Buffer.byteLength(next)>MAX_BYTES) throw new Error('Presentation exceeds 256 KiB'); + const size = Buffer.byteLength(next); + if (size>MAX_BYTES) throw new PresentationRefusal(`Nothing was changed: with this change your tool presentation file would be ${count(size)} bytes, ${count(size - MAX_BYTES)} over its limit of ${count(MAX_BYTES)} (256 KiB). ` + + (field === 'description' ? 'Use a shorter description, or make room by restoring descriptions you no longer need with "description": null.' : 'Make room by restoring descriptions you no longer need with set_tool_description and "description": null.') + + ' Entries for tools no longer available to you can be removed only by whoever runs this host.'); temp = this.config.path + '.' + randomUUID() + '.tmp'; tempFd = openSync(temp,'wx',0o600); writeFileSync(tempFd,next); fchmodSync(tempFd,mode); // preserve permissions on the opened inode, never a replacement path - if (this.read() !== before) throw new Error('File changed concurrently; retry after reading it'); + if (this.read() !== before) throw new PresentationRefusal('Nothing was changed: your tool presentation file changed while this edit was being made. Call this again; it applies to the file as it is now.'); const opened = fstatSync(tempFd), named = lstatSync(temp); if (!named.isFile() || named.dev !== opened.dev || named.ino !== opened.ino) - throw new Error('Temporary file changed concurrently; retry after checking the directory'); + throw new PresentationRefusal('Nothing was changed: the temporary copy this edit was writing was replaced while it was written. Call this again; if it fails the same way, ask whoever runs this host to check what else writes beside your tool presentation file.'); renameSync(temp,this.config.path); temp=undefined; - return {success:true,data:{name:value.name,[field]:value[field],effective:'next newly compiled request',catalogue:this.config.cataloguePath}}; - } catch (error) { return {success:false,isError:true,error:String(error)}; } - finally { if(tempFd!==undefined) closeSync(tempFd); if(temp) try{unlinkSync(temp);}catch{} if(fd!==undefined){closeSync(fd);unlinkSync(lock);} } + result = {success:true,data:{name,[field]:value[field],effective:'next newly compiled request',catalogue:this.config.cataloguePath}}; + } catch (error) { result = {success:false,isError:true,error:editFailure(error)}; } + finally { + // After the rename the edit's result must stand, so no cleanup step may throw. + const tempFdToClose = tempFd, tempToRemove = temp, lockFd = fd; + if (tempFdToClose !== undefined) tidy(() => closeSync(tempFdToClose)); + if (tempToRemove) tidy(() => unlinkSync(tempToRemove)); + if (lockFd !== undefined) { + tidy(() => closeSync(lockFd)); + lockLeft = tidy(() => unlinkSync(lock)); + // A lock already gone was removed by something else, so none is left. + if ((lockLeft?.error as NodeJS.ErrnoException | undefined)?.code === 'ENOENT') lockLeft = undefined; + } + } + if (!lockLeft) return result; + return result.success + ? {...result,data:{...(result.data as Record),note:lockLeftBehind(lockLeft.error)}} + : {...result,error:`${result.error} ${lockLeftBehind(lockLeft.error)}`}; + } + /** + * The name of the tool an edit is for, once the agent's arguments hold up; + * otherwise a refusal naming every problem with them and the call to make. + * The limits a file would refuse (a description's length, the tools that + * can't be hidden) are checked here too, so they read as the agent's input + * rather than as a problem with the file. + */ + private editedName(tool: string, field: 'visible' | 'description', input: unknown, available: ToolDefinition[]): string { + const usage = field === 'visible' + ? `set_tool_visibility takes {"name": a tool's exact name, "visible": true or false}.` + : `set_tool_description takes {"name": a tool's exact name, "description": the new text, or null to restore the default}.`; + if (!input || typeof input !== 'object' || Array.isArray(input)) throw new PresentationRefusal(`Nothing was changed: the arguments were ${shown(input)}, not an object. ${usage}`); + const args = input as Record, problems: string[] = []; + if (!Object.hasOwn(args, 'name')) problems.push('"name" is missing'); + else if (typeof args.name !== 'string') problems.push(`"name" is ${shown(args.name)}, not a string`); + if (!Object.hasOwn(args, field)) problems.push(`"${field}" is missing`); + else if (field === 'visible' ? typeof args.visible !== 'boolean' : args.description !== null && typeof args.description !== 'string') + problems.push(`"${field}" is ${shown(args[field])}, not ${field === 'visible' ? 'true or false' : 'a string or null'}`); + const extra = Object.keys(args).filter((key) => key !== 'name' && key !== field); + if (extra.length) problems.push(`${quotedList(extra)} ${extra.length > 1 ? "aren't arguments" : "isn't an argument"} of ${tool}`); + if (problems.length) throw new PresentationRefusal(`Nothing was changed: ${problems.join('; ')}. ${usage}`); + const name = args.name as string; + if (!available.some((t) => t.name === name)) { + const near = closestNames(name, available.map((t) => t.name)); + throw new PresentationRefusal(`Nothing was changed: no tool named ${quoted(name)} is available to you.${near.length ? ` Did you mean ${quotedList(near, 'or', JSON.stringify)}?` : ''}` + + ` Copy the exact name from your tool catalogue's index, workspace--read {"path":${JSON.stringify(this.config.cataloguePath)},"limit":140}, reading on with "offset" if the index runs past that.`); + } + if (field === 'description' && typeof args.description === 'string' && characters(args.description) > MAX_DESCRIPTION_CHARS) + throw new PresentationRefusal(`Nothing was changed: the description is ${count(characters(args.description))} characters, over the ${count(MAX_DESCRIPTION_CHARS)} allowed. Shorten it and call set_tool_description again.`); + if (field === 'visible' && args.visible === false && protectedNames.has(name)) + throw new PresentationRefusal(`Nothing was changed: ${name} can't be hidden, since workspace--read, set_tool_visibility and set_tool_description are how you read the catalogue and undo these changes. Calling this again won't change that.`); + return name; } } export function renderCatalogue(snapshot: PresentationSnapshot): string { @@ -164,7 +347,8 @@ export function renderCatalogue(snapshot: PresentationSnapshot): string { `Read details: workspace--read {"path":${JSON.stringify(snapshot.cataloguePath)},"offset":LINE,"limit":LINES}.`, 'Line references below belong to this revision; reread the index after changes.', '', ]; - for (const diagnostic of snapshot.diagnostics) lines.push('Diagnostic: '+diagnostic); + // One line each: a quoted parse error can hold the file's own line breaks. + for (const diagnostic of snapshot.diagnostics) lines.push('Diagnostic: '+diagnostic.replace(/\r\n|\r|\n/g, '\\n')); lines.push('## Index by source', ''); const index = new Map(); for (const [source, entries] of groups) { diff --git a/test/closest-names.test.ts b/test/closest-names.test.ts new file mode 100644 index 000000000..15c363a86 --- /dev/null +++ b/test/closest-names.test.ts @@ -0,0 +1,67 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { closestNames, isNearMiss } from '../src/closest-names.js'; + +// What a refusal suggests when a name matches nothing (closestNames), and +// when one name counts as a slip for another (isNearMiss). +const tools = ['workspace--read', 'workspace--ls', 'workspace--write', 'history--read', 'read_image', + 'set_tool_visibility', 'set_tool_description', 'think']; + +test('closestNames ranks a whole segment, then a prefix, then a substring, then a few edits, ignoring case', () => { + assert.deepEqual(closestNames('WORKSPACE--READ', tools), ['workspace--read']); + assert.deepEqual(closestNames('ls', tools), ['workspace--ls'], 'a module tool named without its module'); + assert.deepEqual(closestNames('read', tools, 3), ['history--read', 'workspace--read', 'read_image'], 'segments before a prefix'); + assert.deepEqual(closestNames('set_tool', tools), ['set_tool_visibility', 'set_tool_description']); + assert.deepEqual(closestNames('image', tools), ['read_image']); + assert.deepEqual(closestNames('thnik', tools), ['think'], 'a transposition is one edit'); + assert.deepEqual(closestNames('workspace--raed', tools), ['workspace--read'], 'a typo within a namespace finds its name, not its neighbours'); + assert.deepEqual(closestNames('think', ['thing', 'rethink', 'think_more', 'agent--think'], 4), + ['agent--think', 'think_more', 'rethink', 'thing'], 'each tier ranks ahead of the next, even where the next is fewer edits away'); + assert.deepEqual(closestNames('thinking', ['hinkin', 'think']), ['think', 'hinkin'], + 'a candidate the name starts with ranks ahead of one it holds, even one fewer edits away'); + assert.deepEqual(closestNames('rethinking', ['think']), ['think'], 'a candidate the name holds, however many edits away'); + assert.deepEqual(closestNames('read', ['b--read', 'a--read']), ['b--read', 'a--read'], 'a tie keeps the order given'); + assert.deepEqual(closestNames('think', ['think', 'think']), ['think'], 'a candidate given twice is named once'); + assert.deepEqual(closestNames('refilIntervalM', ['refillIntervalMs']), ['refillIntervalMs'], + 'two edits are within the limit for a long name, past the prefix it shares'); +}); + +test('closestNames gives nothing for a name close to nothing, or for a fragment found in every name', () => { + assert.deepEqual(closestNames('deploy', tools), []); + assert.deepEqual(closestNames('re', tools), [], 'two letters are no prefix or substring match'); + assert.deepEqual(closestNames('read', tools, 1), ['history--read'], 'max bounds the answer'); +}); + +test('isNearMiss: the same ignoring case, or a few edits; past that, a different name', () => { + for (const [slip, field] of [['keyby', 'keyBy'], ['pasthrough', 'passthrough'], ['polices', 'policies'], ['tagsAyn', 'tagsAny'], + ['refilIntervalM', 'refillIntervalMs'], ['channels', 'channel']]) { + assert.equal(isNearMiss(slip, field), true, `${slip} for ${field}`); + } + for (const [other, field] of [['comment', 'content'], ['type', 'text'], ['scope', 'match']]) { + assert.equal(isNearMiss(other, field), false, `${other} is not ${field}`); + } + assert.equal(isNearMiss('workspace--read', 'workspace--write'), false, + 'a shared prefix is a namespace, so neighbours in it are counted by what follows it'); +}); + +test('isNearMiss: a part of a name, of three characters or more, is a slip for it; a name holding another, more edits from it than a slip allows, is not', () => { + for (const [part, field] of [['interval', 'intervalSeconds'], ['mode', 'deliveryMode'], ['channel', 'targetChannelId'], ['EVERY', 'everySeconds'], ['key', 'keyBy']]) { + assert.equal(isNearMiss(part, field), true, `${part} for ${field}`); + } + for (const [extra, field] of [['channel_note', 'channel'], ['pausedReason', 'paused']]) { + assert.equal(isNearMiss(extra, field), false, `${extra} is an extra beside ${field}`); + } + assert.equal(isNearMiss('id', 'channelId'), false, 'two characters are no part of a name'); +}); + +test('closestNames answers a long name that holds every candidate without an edit table for each (a hang guard)', () => { + // A name an agent might paste by mistake, such as a catalogue holding every + // tool name: one name holds the other, so the length difference is their + // distance. An edit table for each would take over a minute here. + const names = Array.from({ length: 100 }, (_, i) => `server${i % 10}--a_tool_whose_name_runs_long_to_cost_the_edit_table_${i}`); + let text = ''; + while (text.length < 1_000_000) text += names.map((name) => `- ${name}: a line of its catalogue entry.`).join('\n') + '\n'; + const started = performance.now(); + assert.deepEqual(closestNames(text, names), [names[10], names[11]], 'the longest names are the closest, in the order given'); + assert.ok(performance.now() - started < 5_000, `took ${Math.round(performance.now() - started)} ms`); +}); diff --git a/test/code-execution.test.ts b/test/code-execution.test.ts index 0b0093d75..0c8b6ad9a 100644 --- a/test/code-execution.test.ts +++ b/test/code-execution.test.ts @@ -300,7 +300,8 @@ describe('PyRunner (real python3)', () => { await new Promise((r) => setTimeout(r, 300)); const second = await runner.exec('print("nope")', []); assert.strictEqual(second.returnCode, 1); - assert.match(second.stderr, /already running/); + assert.strictEqual(second.stderr, "script didn't run: another code_execution script of yours is already running, and they run one at a time. " + + 'Run this one again once that one has finished, or combine the two into one script.'); runner.abort('cleanup'); await first; } finally { @@ -331,7 +332,7 @@ describe('PyRunner (real python3)', () => { } }); - it('fails gracefully when the python binary is missing', async () => { + it('fails gracefully when the python binary is missing, and says code_execution can\'t run here', async () => { const runner = new PyRunner({ pythonPath: '/definitely/not/a/python', onToolCall: async () => '', @@ -339,7 +340,176 @@ describe('PyRunner (real python3)', () => { try { const result = await runner.exec('print(1)', []); assert.strictEqual(result.returnCode, 1); - assert.match(result.stderr, /Failed to start python runtime|exited before becoming ready/); + assert.strictEqual(result.stderr, "script didn't run: this host can't start its Python interpreter, /definitely/not/a/python (ENOENT), " + + "so code_execution can't run scripts here until whoever runs this host installs it or points this host at one. Calling it again before then fails the same way."); + } finally { + runner.dispose(); + } + }); + + it('an interpreter path that is no program, or that runs through a file, says which, and only the first rules out trying again', async () => { + const dir = mkdtempSync(join(tmpdir(), 'pytc-start-')); + const notExecutable = join(dir, 'python3'); + writeFileSync(notExecutable, '#!/bin/sh\n', { mode: 0o644 }); + const noExec = new PyRunner({ pythonPath: notExecutable, onToolCall: async () => '' }); + const throughFile = new PyRunner({ pythonPath: join(notExecutable, 'python3'), onToolCall: async () => '' }); + try { + assert.strictEqual((await noExec.exec('print(1)', [])).stderr, `script didn't run: this host can't start its Python interpreter, ${notExecutable} (EACCES), ` + + "so code_execution can't run scripts here until whoever runs this host makes it executable or points this host at another. Calling it again before then fails the same way."); + assert.strictEqual((await throughFile.exec('print(1)', [])).stderr, `script didn't run: the Python interpreter, ${join(notExecutable, 'python3')}, ` + + "couldn't be started (ENOTDIR), so none of the script ran. Run it again; if it fails the same way, ask whoever runs this host."); + } finally { + noExec.dispose(); + throughFile.dispose(); + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("a runtime whose files can't be written says so, with the error's path cut at 500 characters, not that the interpreter failed", async () => { + const dir = mkdtempSync(join(tmpdir(), 'pytc-files-')); + // Nested past 500 characters, so Node's message, which quotes the path, is cut. + const missing = join(dir, ...Array.from({ length: 6 }, () => 'm'.repeat(90))); + const saved = process.env.TMPDIR; + process.env.TMPDIR = missing; + const runner = new PyRunner({ onToolCall: async () => '' }); + try { + const result = await runner.exec('print(1)', []); + assert.strictEqual(result.returnCode, 1); + const message = `ENOENT: no such file or directory, mkdtemp '${join(missing, 'af-pytc-XXXXXX')}'`; + assert.ok(message.length > 500, 'the message is long enough to be cut'); + assert.strictEqual(result.stderr, "script didn't run: this host couldn't create or write the files its Python runtime starts from (" + + `${message.slice(0, 500)}… (${message.length} characters)), so none of the script ran. Run it again; if it fails the same way, ask whoever runs this host.`); + } finally { + if (saved === undefined) delete process.env.TMPDIR; else process.env.TMPDIR = saved; + runner.dispose(); + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('an interpreter that exits as it starts: nothing in the script ran, and one more try is worth it', async () => { + // Node, given the runtime's Python source, exits on its syntax at once. + const runner = new PyRunner({ pythonPath: process.execPath, onToolCall: async () => '' }); + try { + const result = await runner.exec('print(1)', []); + assert.strictEqual(result.returnCode, 1); + assert.strictEqual(result.stderr, `script didn't run: the Python interpreter, ${process.execPath}, exited as it started, so none of the script ran. ` + + 'Run it again; if it fails the same way, ask whoever runs this host.'); + } finally { + runner.dispose(); + } + }); + + it('an interpreter that exits under a script says the rest didn\'t run and its state is gone', async () => { + const runner = new PyRunner({ onToolCall: async () => '' }); + try { + assert.strictEqual((await runner.exec('x = 41', [])).returnCode, 0); + const result = await runner.exec('import os\nos._exit(3)', []); + assert.strictEqual(result.returnCode, 1); + assert.strictEqual(result.stderr, "script stopped: the Python interpreter exited with code 3 while it ran, so the rest of the script didn't run, " + + 'and variables from earlier scripts are gone. Tool calls it had already made keep their effects, so check them before you run it again in a new interpreter. ' + + 'If it stops the same way, check whether the script itself ends the process, for example with os._exit(); if not, ask whoever runs this host.'); + assert.match((await runner.exec('print(x)', [])).stderr, /NameError: name 'x' is not defined/); + const killed = await runner.exec('import os, signal\nos.kill(os.getpid(), signal.SIGKILL)', []); + assert.strictEqual(killed.stderr, "script stopped: the Python interpreter was ended by SIGKILL while it ran, so the rest of the script didn't run, " + + 'and variables from earlier scripts are gone. Tool calls it had already made keep their effects, so check them before you run it again in a new interpreter. ' + + 'If it stops the same way, check whether the script itself ends the process, for example by using too much memory; if not, ask whoever runs this host.'); + } finally { + runner.dispose(); + } + }); + + it('a tool function given keyword arguments or a non-dict is not called, and says the dict to write', async () => { + const calls: string[] = []; + const runner = new PyRunner({ onToolCall: async (name) => { calls.push(name); return ''; } }); + try { + const result = await runner.exec( + [ + 'class Loud(str):', + ' def __repr__(self):', + ' raise RuntimeError("repr ran")', + 'class Int(int):', + ' def __repr__(self):', + ' raise RuntimeError("repr ran")', + 'class Meta(type):', + ' def __eq__(cls, other):', + ' raise RuntimeError("metaclass __eq__ ran")', + ' __hash__ = type.__hash__', + ' @property', + ' def __name__(cls):', + ' raise RuntimeError("metaclass __name__ ran")', + 'class Odd(metaclass=Meta):', + ' def __repr__(self):', + ' raise RuntimeError("repr ran")', + 'for call in (lambda: test__echo(message="alpha", limit=5, items=[1]), lambda: test__echo({"message": "alpha"}, limit=5), lambda: test__echo("alpha"),', + ' lambda: test__echo("alpha", limit=5),', + ' lambda: test__echo(text=Loud("x"), n=Int(3)), lambda: test__echo({Int(4): "a"}, n=1),', + ' lambda: test__echo({"k" * 61: 1, "j" * 60: 2}, n=1), lambda: test__echo(Odd(), n=1)):', + ' try:', + ' await call()', + ' except TypeError as e:', + ' print(e)', + ].join('\n'), + ECHO_TOOLS, + ); + assert.strictEqual(result.returnCode, 0, result.stderr); + assert.deepStrictEqual(result.stdout.trimEnd().split('\n'), [ + "test__echo wasn't called: a tool function takes one dict of arguments, not keyword arguments, so write await test__echo({'message': , 'limit': , 'items': })", + "test__echo wasn't called: a tool function takes one dict of arguments, not keyword arguments, so write await test__echo({'message': , 'limit': })", + "test__echo wasn't called: a tool function takes one dict of arguments, such as await test__echo({...}), and this call gave it ", + "test__echo wasn't called: a tool function takes one dict of arguments, not keyword arguments, and this call also gave it , " + + 'so put every argument in one dict: await test__echo({...})', + // A value of a subclass is shown by its type: its own __repr__ never runs. + "test__echo wasn't called: a tool function takes one dict of arguments, not keyword arguments, so write await test__echo({'text': , 'n': })", + // A key that isn't a str is shown as a value is, so its __repr__ doesn't run either. + "test__echo wasn't called: a tool function takes one dict of arguments, not keyword arguments, so write await test__echo({: , 'n': })", + // A str key is shown whole up to 60 characters, and past that by its type. + "test__echo wasn't called: a tool function takes one dict of arguments, not keyword arguments, so write await test__echo(" + + `{: , '${'j'.repeat(60)}': , 'n': })`, + // A class's metaclass code doesn't run either: its type is named through type's own descriptor. + "test__echo wasn't called: a tool function takes one dict of arguments, not keyword arguments, and this call also gave it , " + + 'so put every argument in one dict: await test__echo({...})', + ]); + assert.deepStrictEqual(calls, []); + } finally { + runner.dispose(); + } + }); + + it('a tool function takes args= on its own as the dict, and None either way as no arguments, as it always has', async () => { + const calls: unknown[] = []; + const runner = new PyRunner({ onToolCall: async (_name, args) => { calls.push(args); return 'ok'; } }); + try { + const result = await runner.exec([ + 'await test__echo(None)', + 'await test__echo(args=None)', + 'await test__echo(args={"message": "a"})', + 'await test__echo({"message": "b"})', + ].join('\n'), ECHO_TOOLS); + assert.strictEqual(result.returnCode, 0, result.stderr); + assert.deepStrictEqual(calls, [{}, {}, { message: 'a' }, { message: 'b' }]); + } finally { + runner.dispose(); + } + }); + + it('a keyword named args beside other keywords stays in the dict to write, and two positional arguments are refused', async () => { + const runner = new PyRunner({ onToolCall: async () => 'called' }); + try { + const result = await runner.exec([ + 'for call in (lambda: test__echo(message="alpha", args={"limit": 5}), lambda: test__echo({"message": "alpha"}, args={"limit": 5}),', + ' lambda: test__echo(args="notes.md"), lambda: test__echo({"a": 1}, {"b": 2})):', + ' try:', + ' await call()', + ' except TypeError as e:', + ' print(e)', + ].join('\n'), ECHO_TOOLS); + assert.strictEqual(result.returnCode, 0, result.stderr); + assert.deepStrictEqual(result.stdout.trimEnd().split('\n'), [ + "test__echo wasn't called: a tool function takes one dict of arguments, not keyword arguments, so write await test__echo({'message': , 'args': })", + "test__echo wasn't called: a tool function takes one dict of arguments, not keyword arguments, so write await test__echo({'message': , 'args': })", + "test__echo wasn't called: a tool function takes one dict of arguments, such as await test__echo({...}), and this call gave it ", + "test__echo wasn't called: a tool function takes one dict of arguments, such as await test__echo({...}), and this call gave it 2 positional arguments", + ]); } finally { runner.dispose(); } @@ -561,7 +731,8 @@ describe('PyRunner: an abort or dispose while the interpreter starts (real pytho assert.strictEqual(result.aborted, true); assert.strictEqual( result.stderr, - `Failed to start python runtime (${exits.path}): python runtime exited before becoming ready`, + `script didn't run: the Python interpreter, ${exits.path}, exited as it started, so none of the script ran. ` + + 'Run it again; if it fails the same way, ask whoever runs this host.', ); } finally { runner.dispose(); @@ -1136,6 +1307,38 @@ describe('framework background scripts + spill (real python3)', () => { } }); + it('a background script stopped at its lifetime is told that is the longest one runs here', async () => { + const { tempDir, storePath } = tempStorePath('pytc-bglimit-'); + const membrane = new MockMembrane(); + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'noted' }])); + const framework = await createBgFramework(storePath, membrane, { codeExecution: { backgroundMaxLifetimeMs: 1000 } }); + try { + framework.start(); + const started = await framework.executeToolCall({ + id: 'l1', + name: 'code_execution', + input: { code: 'import asyncio\nawait asyncio.sleep(60)', background: true }, + callerAgentName: 'prime', + }); + assert.strictEqual(started.success, true, JSON.stringify(started)); + const deadline = Date.now() + 20_000; + let stopMessage: { content: ContentBlockLike[] } | null = null; + while (Date.now() < deadline && !stopMessage) { + await new Promise((r) => setTimeout(r, 100)); + const cm = framework.getAgent('prime')?.getContextManager(); + const msgs = (cm?.queryMessages({}).messages ?? []) as unknown as Array<{ content: ContentBlockLike[]; metadata?: { source?: string } }>; + stopMessage = msgs.find((m) => m.metadata?.source === 'background-script') ?? null; + } + assert.ok(stopMessage, 'the stop should wake the agent'); + const text = (stopMessage.content[0] as { text?: string }).text ?? ''; + assert.match(text, /script stopped: it reached its 1s time limit\./); + assert.match(text, /1s is the longest a background script runs here; starting it again runs it from the top\./, 'the background ceiling reaches the message'); + } finally { + await framework.stop(); + rmSync(tempDir, { recursive: true, force: true }); + } + }); + it('oversized tool results spill to a workspace file with a reference', async () => { const { tempDir, storePath } = tempStorePath('pytc-spill-'); const mountDir = join(tempDir, 'mount'); @@ -1259,6 +1462,37 @@ describe('code_execution per-call time limit (time_limit_ms)', () => { } }); + // On a deployment without maxScriptTimeoutMs the ceiling is the default, and a + // larger time_limit_ms would only buy a second run that stops the same way. + it('PyRunner: a script stopped at its limit is offered a longer one only below the ceiling', async () => { + const code = 'import asyncio\nawait asyncio.sleep(60)'; + const effects = 'Tool calls it had already made keep their effects, so check them before running it again.'; + const runner = new PyRunner({ onToolCall: async () => '' }); + const background = new PyRunner({ onToolCall: async () => '', idleReclaimMs: 0 }); + const backgroundBelow = new PyRunner({ onToolCall: async () => '', idleReclaimMs: 0 }); + try { + const below = await runner.exec(code, [], undefined, { deadlineMs: 1000, ceilingMs: 5000 }); + assert.ok(below.stderr.endsWith(`script stopped: it reached its 1s time limit. ${effects} To give it longer, pass time_limit_ms up to 5000 (5s).\n`), below.stderr); + const at = await runner.exec(code, [], undefined, { deadlineMs: 1000, ceilingMs: 1000 }); + assert.ok(at.stderr.endsWith(`${effects} 1s is the longest a script can run here, so split the work across shorter scripts.\n`), at.stderr); + const longerInBackground = await runner.exec(code, [], undefined, { deadlineMs: 1000, ceilingMs: 1000, backgroundCeilingMs: 86_400_000 }); + assert.ok(longerInBackground.stderr.endsWith(`${effects} 1s is the longest a script can run here, so split the work across shorter scripts, ` + + 'or run it with background: true, where a script can run up to 24 h.\n'), longerInBackground.stderr); + const noLongerInBackground = await runner.exec(code, [], undefined, { deadlineMs: 1000, ceilingMs: 1000, backgroundCeilingMs: 1000 }); + assert.ok(noLongerInBackground.stderr.endsWith('so split the work across shorter scripts.\n'), noLongerInBackground.stderr); + const unknown = await runner.exec(code, [], undefined, { deadlineMs: 1000 }); + assert.ok(unknown.stderr.endsWith(`script stopped: it reached its 1s time limit. ${effects}\n`), unknown.stderr); + const watcher = await background.exec(code, [], { logPath: null, lifetimeMs: 1000, onWake: async () => null }, { ceilingMs: 1000 }); + assert.ok(watcher.stderr.endsWith(`${effects} 1s is the longest a background script runs here; starting it again runs it from the top.\n`), watcher.stderr); + const shortWatcher = await backgroundBelow.exec(code, [], { logPath: null, lifetimeMs: 1000, onWake: async () => null }, { ceilingMs: 5000 }); + assert.ok(shortWatcher.stderr.endsWith(`${effects} To give it longer, start it with time_limit_ms up to 5000 (5s).\n`), shortWatcher.stderr); + } finally { + runner.dispose(); + background.dispose(); + backgroundBelow.dispose(); + } + }); + it('PyRunner: a script that catches the cancellation and finishes is not reported as stopped', async () => { const runner = new PyRunner({ onToolCall: async () => '' }); try { @@ -1305,12 +1539,13 @@ describe('code_execution per-call time limit (time_limit_ms)', () => { async function withFramework( codeExecution: Record, body: (framework: AgentFramework) => Promise, + agents: Parameters[0]['agents'] = [], ): Promise { const { tempDir, storePath } = tempStorePath('pytc-timeout-'); const framework = await AgentFramework.create({ storePath, membrane: new MockMembrane().asMembrane(), - agents: [], + agents, modules: [new ScriptToolModule()], syncIntervalMs: 0, codeExecution: { enabled: true, ...codeExecution }, @@ -1341,6 +1576,7 @@ describe('code_execution per-call time limit (time_limit_ms)', () => { const short = dataOf(await run(framework, { code: 'import asyncio\nawait asyncio.sleep(60)', time_limit_ms: 1000 })); assert.strictEqual(short.return_code, 1); assert.match(short.stderr, /reached its 1s time limit/); + assert.match(short.stderr, /To give it longer, pass time_limit_ms up to 600000 \(10 min\)\.\n$/, 'the deployment ceiling reaches the message'); const normal = dataOf(await run(framework, { code: 'import asyncio\nawait asyncio.sleep(1.5)\nprint("done")' })); assert.strictEqual(normal.return_code, 0); assert.match(normal.stdout, /done/); @@ -1353,10 +1589,36 @@ describe('code_execution per-call time limit (time_limit_ms)', () => { const r = dataOf(await run(framework, { code: 'import asyncio\nawait asyncio.sleep(60)', time_limit_ms: 60_000 })); assert.strictEqual(r.return_code, 1); assert.strictEqual(r.time_limit_note, "time_limit_ms 60000 was capped at 1000, this deployment's maximum"); + assert.match(r.stderr, /1s is the longest a script can run here, so split the work across shorter scripts, or run it with background: true, where a script can run up to 24 h\.\n$/, + "no larger limit is offered at the ceiling, and the deployment's background lifetime reaches the message"); assert.ok(Date.now() - started < 15_000, 'the cap was not applied'); }); }); + it('at the ceiling, background mode is offered only to an agent that could start a background script', async () => { + const atCeiling = (framework: AgentFramework, callerAgentName?: string) => framework.executeToolCall({ + id: `ce-${Math.random()}`, name: 'code_execution', input: { code: 'import asyncio\nawait asyncio.sleep(60)' }, ...(callerAgentName ? { callerAgentName } : {}), + }); + const noOffer = /1s is the longest a script can run here, so split the work across shorter scripts\.\n$/; + // Background scripts are the primary agent's. + await withFramework({ scriptTimeoutMs: 1000 }, async (framework) => { + assert.match(dataOf(await atCeiling(framework, 'prime')).stderr, /, or run it with background: true, where a script can run up to 24 h\.\n$/); + assert.match(dataOf(await atCeiling(framework, 'other')).stderr, noOffer); + }, [{ name: 'prime', model: 'test-model', systemPrompt: 'You are prime.', allowedTools: 'all' }]); + // A deployment that allows none offers none. + await withFramework({ scriptTimeoutMs: 1000, maxBackgroundScripts: 0 }, async (framework) => { + assert.match(dataOf(await atCeiling(framework)).stderr, noOffer); + }); + // Nor does an agent whose background slots are all taken. + await withFramework({ scriptTimeoutMs: 1000, maxBackgroundScripts: 1 }, async (framework) => { + const started = await framework.executeToolCall({ + id: 'bg-1', name: 'code_execution', input: { code: 'import asyncio\nawait asyncio.sleep(60)', background: true }, callerAgentName: 'prime', + }); + assert.strictEqual(started.success, true, JSON.stringify(started)); + assert.match(dataOf(await atCeiling(framework, 'prime')).stderr, noOffer); + }, [{ name: 'prime', model: 'test-model', systemPrompt: 'You are prime.', allowedTools: 'all' }]); + }); + it('maxScriptTimeoutMs lets an agent ask for longer than the default', async () => { await withFramework({ scriptTimeoutMs: 1000, maxScriptTimeoutMs: 10_000 }, async (framework) => { const code = 'import asyncio\nawait asyncio.sleep(2)\nprint("long done")'; @@ -1615,12 +1877,13 @@ describe('code_execution time limit vs blocking code (real python3)', { skip: pr assert.strictEqual((await runner.exec('x = 41', [])).returnCode, 0); const code = 'import time\ntry:\n time.sleep(60)\nexcept KeyboardInterrupt:\n time.sleep(60)'; const started = Date.now(); - const killed = await runner.exec(code, [], undefined, { deadlineMs: 1000 }); + const killed = await runner.exec(code, [], undefined, { deadlineMs: 1000, ceilingMs: 2000 }); assert.strictEqual(killed.aborted, true); assert.strictEqual(killed.returnCode, 1); - assert.match( + assert.strictEqual( killed.stderr, - /script stopped: it reached its 1s time limit and did not respond, so it was killed and the interpreter restarted \(variables from earlier scripts are gone\)/, + 'script stopped: it reached its 1s time limit and did not respond, so it was killed and the interpreter restarted (variables from earlier scripts are gone). ' + + 'Tool calls it had already made keep their effects, so check them before running it again. To give it longer, pass time_limit_ms up to 2000 (2s).', ); assert.ok(Date.now() - started < 5000, `killed after the grace (${Date.now() - started}ms)`); const after = await runner.exec('print(x)', []); @@ -1631,6 +1894,35 @@ describe('code_execution time limit vs blocking code (real python3)', { skip: pr } }); + it('a background script killed at its lifetime is not told of an interpreter it never shared', async () => { + const runner = new PyRunner({ onToolCall: async () => '', cancelGraceMs: 500, idleReclaimMs: 0 }); + try { + const code = 'import time\ntry:\n time.sleep(60)\nexcept KeyboardInterrupt:\n time.sleep(60)'; + const killed = await runner.exec(code, [], { logPath: null, lifetimeMs: 1000, onWake: async () => null }, { ceilingMs: 1000 }); + assert.strictEqual(killed.aborted, true); + assert.strictEqual( + killed.stderr, + 'script stopped: it reached its 1s time limit and did not respond, so it was killed. Tool calls it had already made keep their effects, ' + + 'so check them before running it again. 1s is the longest a background script runs here; starting it again runs it from the top.', + ); + } finally { + runner.dispose(); + } + }); + + it('a background script whose interpreter exits under it is not told of earlier scripts\' variables', async () => { + const runner = new PyRunner({ onToolCall: async () => '' }); + try { + const exited = await runner.exec('import os\nos._exit(3)', [], { logPath: null, lifetimeMs: 60_000, onWake: async () => null }); + assert.strictEqual(exited.returnCode, 1); + assert.strictEqual(exited.stderr, "script stopped: the Python interpreter exited with code 3 while it ran, so the rest of the script didn't run. " + + 'Tool calls it had already made keep their effects, so check them before you run it again in a new interpreter. ' + + 'If it stops the same way, check whether the script itself ends the process, for example with os._exit(); if not, ask whoever runs this host.'); + } finally { + runner.dispose(); + } + }); + it('the time-limit note starts on a line of its own after an interpreter exit, whose text ends without one', async () => { const runner = new PyRunner({ onToolCall: async () => '' }); try { @@ -1640,7 +1932,11 @@ describe('code_execution time limit vs blocking code (real python3)', { skip: pr assert.strictEqual(result.returnCode, 1); assert.strictEqual( result.stderr, - 'python runtime exited unexpectedly (code=3, signal=none)\nscript stopped: it reached its 1s time limit\n', + "script stopped: the Python interpreter exited with code 3 while it ran, so the rest of the script didn't run, " + + 'and variables from earlier scripts are gone. Tool calls it had already made keep their effects, so check them before you run it again ' + + 'in a new interpreter. If it stops the same way, check whether the script itself ends the process, for example with os._exit(); ' + + 'if not, ask whoever runs this host.\n' + + 'script stopped: it reached its 1s time limit. Tool calls it had already made keep their effects, so check them before running it again.\n', ); } finally { runner.dispose(); diff --git a/test/tool-presentation.test.ts b/test/tool-presentation.test.ts index 63b121883..092ec448e 100644 --- a/test/tool-presentation.test.ts +++ b/test/tool-presentation.test.ts @@ -2,7 +2,7 @@ import fs from 'node:fs'; import {syncBuiltinESMExports} from 'node:module'; import {test} from 'node:test'; import assert from 'node:assert/strict'; -import {mkdtempSync,rmSync,writeFileSync,readFileSync,chmodSync,statSync,existsSync} from 'node:fs'; +import {mkdtempSync,rmSync,writeFileSync,readFileSync,chmodSync,statSync,existsSync,mkdirSync} from 'node:fs'; import {tmpdir} from 'node:os'; import {join} from 'node:path'; import {ToolPresentation,presentationTools,renderCatalogue} from '../src/tool-presentation.js'; @@ -216,7 +216,286 @@ test('replaced temporary pathname cannot redirect permission changes',()=>{ assert.equal(statSync(victim).mode&0o777,0o600,'private target mode unchanged'); assert.equal(readFileSync(victim,'utf8'),'private'); assert.equal(result.success,false,'detected replacement refuses commit'); + assert.equal(result.error,'Nothing was changed: the temporary copy this edit was writing was replaced while it was written. Call this again; if it fails the same way, ask whoever runs this host to check what else writes beside your tool presentation file.'); assert.equal(readFileSync(path,'utf8'),before); assert.deepEqual(fs.readdirSync(dir).sort(),['private','tools.json']); } finally {fs.writeFileSync=originalWrite;syncBuiltinESMExports();rmSync(dir,{recursive:true,force:true});} }); + +// What an agent reads when an edit is refused, and what +// the catalogue says about a file the host can't use. Each refusal leads with +// "Nothing was changed", names every problem it can see, and says what helps. +const presentationTargets=(...names:string[])=>names.map(name=>({name,description:'original',inputSchema:{type:'object' as const}})); + +test('an edit refusal names every problem with its arguments, the call that works, and the closest names',()=>{ + const dir=mkdtempSync(join(tmpdir(),'presentation-refusals-')); + try { + const path=join(dir,'tools.json'), p=new ToolPresentation({path,cataloguePath:'board/tools.md'}); + const tools=[...presentationTargets('example','workspace--read'),...presentationTools('board/tools.md')]; + const refusal=(tool:string,input:unknown)=>{const r=p.edit(tool,input,tools);assert.equal(r.success,false);assert.equal(r.isError,true);return r.error;}; + assert.equal(refusal('set_tool_visibility',{nmae:'example',visible:'no'}), + 'Nothing was changed: "name" is missing; "visible" is "no", not true or false; "nmae" isn\'t an argument of set_tool_visibility. ' + +'set_tool_visibility takes {"name": a tool\'s exact name, "visible": true or false}.'); + assert.equal(refusal('set_tool_description',['example']), + 'Nothing was changed: the arguments were an array, not an object. ' + +'set_tool_description takes {"name": a tool\'s exact name, "description": the new text, or null to restore the default}.'); + assert.equal(refusal('set_tool_visibility',{name:'exampel',visible:false}), + 'Nothing was changed: no tool named "exampel" is available to you. Did you mean "example"? Copy the exact name from your tool catalogue\'s index, ' + +'workspace--read {"path":"board/tools.md","limit":140}, reading on with "offset" if the index runs past that.'); + // A suggested name is quoted whole, however long, since it's what the agent copies. + const long='mcpl--'+'research_assistant_'.repeat(3)+'search_archive'; + const withLong=[...tools,...presentationTargets(long)]; + const r=p.edit('set_tool_visibility',{name:long.slice(0,-1),visible:false},withLong); + assert.ok(long.length>60); + assert.match(r.error!,new RegExp(`Did you mean ${JSON.stringify(long)}\\?`)); + // The name the agent typed is quoted as a file's names are: whole up to 60 characters, and past that cut, with its length. + assert.ok(r.error!.startsWith(`Nothing was changed: no tool named ${JSON.stringify(long.slice(0,57))}... (76 characters) is available to you.`),r.error); + assert.doesNotMatch(refusal('set_tool_visibility',{name:'deploy',visible:false})!,/Did you mean/,'a name close to nothing gets no guess'); + // Two close names are both offered, the closer first. + const two=p.edit('set_tool_visibility',{name:'exampl',visible:false},[...tools,...presentationTargets('example_two')]); + assert.match(two.error!,/ Did you mean "example" or "example_two"\? /); + // These two limits are the file's rules too; the agent reads them as its own input. + assert.equal(refusal('set_tool_description',{name:'example',description:'x'.repeat(32769)}), + 'Nothing was changed: the description is 32,769 characters, over the 32,768 allowed. Shorten it and call set_tool_description again.'); + assert.equal(refusal('set_tool_description',{name:'example',description:'\u{1F600}'.repeat(32769)}), + 'Nothing was changed: the description is 32,769 characters, over the 32,768 allowed. Shorten it and call set_tool_description again.', + 'an emoji of one code point counts once'); + assert.equal(refusal('set_tool_description',{name:'example',description:5}), + 'Nothing was changed: "description" is 5, not a string or null. ' + +'set_tool_description takes {"name": a tool\'s exact name, "description": the new text, or null to restore the default}.'); + for (const name of ['workspace--read','set_tool_visibility','set_tool_description']) assert.equal(refusal('set_tool_visibility',{name,visible:false}), + `Nothing was changed: ${name} can't be hidden, since workspace--read, set_tool_visibility and set_tool_description are how you read the catalogue and undo these changes. Calling this again won't change that.`); + assert.equal(existsSync(path),false,'no refusal wrote the file'); + assert.equal(p.edit('set_tool_description',{name:'example',description:'\u{1F600}'.repeat(32768)},tools).success,true,'at the limit in characters, though twice it in UTF-16 units'); + } finally {rmSync(dir,{recursive:true,force:true});} +}); + +test('a presentation file the host can\'t use: edits refuse it and the catalogue says so, each naming every problem',()=>{ + const dir=mkdtempSync(join(tmpdir(),'presentation-file-problems-')); + try { + const path=join(dir,'tools.json'), p=new ToolPresentation({path,cataloguePath:'board/tools.md'}); + const tools=presentationTargets('example','workspace--read'); + const broken=JSON.stringify({version:2,tools:{example:{visible:'no',hidden:true},'workspace--read':{visible:false}},note:'x'}); + writeFileSync(path,broken); + const problems='"version" must be 1, and it is 2; only "version" and "tools" belong at the top level, not "note"; ' + +'the entry for "example" has "hidden", and an entry may have only "description" and "visible"; ' + +'the entry for "example" has "visible": "no", which must be true or false; ' + +'the entry for "workspace--read" hides it, which isn\'t allowed, since workspace--read, set_tool_visibility and set_tool_description are how the catalogue is read and how these settings are undone'; + assert.equal(p.edit('set_tool_visibility',{name:'example',visible:true},tools).error, + `Nothing was changed: your tool presentation file can't be used (${problems}), and these tools won't write over it until it's repaired. Ask whoever runs this host to repair it.`); + assert.equal(readFileSync(path,'utf8'),broken); + assert.deepEqual(p.resolve(tools).diagnostics,[`Your tool presentation file can't be used (${problems}), so none of its settings apply: ` + +'every tool is visible and shows its configured default description, or its installed one where none is configured. ' + +'set_tool_visibility and set_tool_description won\'t change the file until it\'s repaired; ask whoever runs this host to repair it.']); + writeFileSync(path,'{"version":1,'); + assert.match(p.edit('set_tool_visibility',{name:'example',visible:true},tools).error!, + /^Nothing was changed: your tool presentation file can't be used \(it isn't valid JSON: .+\), and these tools won't write over it/); + writeFileSync(path,'[]'); + const notObject=p.resolve(tools).diagnostics[0]; + assert.ok(notObject.startsWith('Your tool presentation file can\'t be used (its top level is an array, not an object like {"version":1,"tools":{}}), so none of its settings apply'),notObject); + // A long value is quoted short, and past ten problems the rest are counted. + writeFileSync(path,JSON.stringify({version:1,tools:Object.fromEntries(Array.from({length:12},(_,i)=>[`t${i}`,{visible:'y'.repeat(50)}]))})); + const many=p.resolve(tools).diagnostics[0]; + assert.match(many,/\(the entry for "t0" has "visible": "y{36}\.\.\., which must be true or false; /); + assert.match(many,/the entry for "t9" has [^;]*; 2 more problems\), so none of its settings apply/); + writeFileSync(path,JSON.stringify({version:1,tools:Object.fromEntries(Array.from({length:11},(_,i)=>[`t${i}`,{visible:'y'}]))})); + assert.match(p.resolve(tools).diagnostics[0],/; 1 more problem\), so none of its settings apply/); + // A name of any length is quoted cut, with its length. + writeFileSync(path,JSON.stringify({version:1,tools:{['k'.repeat(100)]:{visible:'no'}}})); + assert.match(p.resolve(tools).diagnostics[0],/\(the entry for "k{57}"\.\.\. \(100 characters\) has "visible": "no", which must be true or false\)/); + // Names and values are counted and cut in code points, so a cut never leaves half of a surrogate pair. + writeFileSync(path,JSON.stringify({version:1,tools:{['\u{1F600}'.repeat(60)]:{visible:'no'},['\u{1F600}'.repeat(61)]:{visible:'\u{1F600}'.repeat(40)}}})); + const emoji=p.resolve(tools).diagnostics[0]; + assert.ok(emoji.includes(`the entry for "${'\u{1F600}'.repeat(60)}" has`),'60 emoji are quoted whole'); + assert.ok(emoji.includes(`the entry for "${'\u{1F600}'.repeat(57)}"... (61 characters) has "visible": "${'\u{1F600}'.repeat(36)}...,`),'past 60 cut at 57, a value at 37'); + assert.doesNotMatch(emoji,/[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?l.startsWith('Diagnostic: ')); + assert.equal(lines.length,4); + assert.match(lines[0],/^Diagnostic: Your tool presentation file can't be used \(it isn't valid JSON: .*\), so none of its settings apply: .* ask whoever runs this host to repair it\.$/); + assert.deepEqual(lines.slice(1),['Diagnostic: first\\nsecond','Diagnostic: third\\nfourth','Diagnostic: fifth\\nsixth']); + } finally {rmSync(dir,{recursive:true,force:true});} +}); + +test('description defaults that set visibility, are missing or are configured twice are each said, and the rest still apply',()=>{ + const dir=mkdtempSync(join(tmpdir(),'presentation-defaults-problems-')); + try { + const web=join(dir,'web.json'), good=join(dir,'good.json'), absent=join(dir,'absent.json'); + writeFileSync(web,JSON.stringify({version:1,tools:{'web--fetch':{description:'fetch',visible:false},'web--post':{visible:true}}})); + writeFileSync(good,JSON.stringify({version:1,tools:{'workspace--read':{description:'from defaults'}}})); + const p=new ToolPresentation({path:join(dir,'tools.json'),cataloguePath:'board/tools.md',defaults:[ + {source:'MCPL server: web',path:web},{source:'Module: workspace',path:good},{source:'Module: workspace',path:absent},{source:'MCPL server: mail',path:absent}]}); + const tools=presentationTargets('web--fetch','web--post','workspace--read','mail--send'); + const sources=new Map([['web--fetch','MCPL server: web'],['web--post','MCPL server: web'],['workspace--read','Module: workspace'],['mail--send','MCPL server: mail']]); + const snapshot=p.resolve(tools,sources); + const fallback='so its tools show their installed descriptions, or yours where you set one. Ask whoever runs this host to fix that defaults file.'; + assert.deepEqual(snapshot.diagnostics,[ + `The description defaults for "MCPL server: web" can't be used (it sets "visible" for "web--fetch" and "web--post", and a defaults file may set only descriptions, since visibility is yours to choose), ${fallback}`, + 'The description defaults for "Module: workspace" are configured twice; only the first is read, and the other is ignored. Ask whoever runs this host to remove the duplicate.', + `The description defaults for "MCPL server: mail" can't be used (it doesn't exist), ${fallback}`, + ]); + assert.deepEqual(snapshot.available.map(t=>t.description),['original','original','from defaults','original']); + } finally {rmSync(dir,{recursive:true,force:true});} +}); + +test('a stale lock, a linked file, a full file, a change underneath and a failed write each say what happened and change nothing, and a lock left after a done edit leaves it done, with a note',()=>{ + const dir=mkdtempSync(join(tmpdir(),'presentation-conditions-')); + const originalWrite=fs.writeFileSync, originalRename=fs.renameSync, originalUnlink=fs.unlinkSync; + try { + const path=join(dir,'tools.json'), p=new ToolPresentation({path,cataloguePath:'board/tools.md'}); + const tools=presentationTargets('example'); + const hide=()=>p.edit('set_tool_visibility',{name:'example',visible:false},tools).error; + writeFileSync(path+'.lock',''); + assert.equal(hide(),'Nothing was changed: an earlier edit of your tool presentation file left its lock behind, or another edit is in progress. ' + +'Call this again in a few seconds. If it fails the same way, ask whoever runs this host to remove the stale lock, the presentation file\'s name with .lock added, once nothing is editing the file.'); + fs.unlinkSync(path+'.lock'); + const target=join(dir,'target.json');writeFileSync(target,'{"version":1,"tools":{}}');fs.symlinkSync(target,path); + assert.equal(hide(),'Nothing was changed: your tool presentation file is a symbolic link, and these tools don\'t write through links. ' + +'Ask whoever runs this host to make the change in the file it points to, or to replace the link with that file.'); + assert.equal(readFileSync(target,'utf8'),'{"version":1,"tools":{}}'); + fs.unlinkSync(path); + // A file a little under its limit, which one more long description would take over it. + const fillers=Object.fromEntries(Array.from({length:8},(_,i)=>[`filler-${i}`,{description:'x'.repeat(32000)}])); + const full=JSON.stringify({version:1,tools:fillers},null,2)+'\n'; + writeFileSync(path,full); + const long='y'.repeat(32000); + const size=Buffer.byteLength(JSON.stringify({version:1,tools:{...fillers,example:{description:long}}},null,2)+'\n'); + assert.ok(Buffer.byteLength(full)<256*1024 && size>256*1024,'the file fits, and the edit would not'); + assert.equal(p.edit('set_tool_description',{name:'example',description:long},tools).error, + `Nothing was changed: with this change your tool presentation file would be ${size.toLocaleString('en-US')} bytes, ${(size-256*1024).toLocaleString('en-US')} over its limit of 262,144 (256 KiB). ` + +'Use a shorter description, or make room by restoring descriptions you no longer need with "description": null. ' + +'Entries for tools no longer available to you can be removed only by whoever runs this host.'); + assert.equal(readFileSync(path,'utf8'),full); + // A file a few bytes under its limit, which hiding one more tool would take over it. + const nine=Object.fromEntries(Array.from({length:9},(_,i)=>[`filler-${i}`,{description:'x'.repeat(29000)}])); + const filled=(n:number)=>JSON.stringify({version:1,tools:{...nine,'filler-8':{description:'x'.repeat(n)}}},null,2)+'\n'; + const nearlyFull=filled(29000+256*1024-Buffer.byteLength(filled(29000))-10); + writeFileSync(path,nearlyFull); + const hidden=Buffer.byteLength(JSON.stringify({version:1,tools:{...JSON.parse(nearlyFull).tools,example:{visible:false}}},null,2)+'\n'); + assert.ok(Buffer.byteLength(nearlyFull)<=256*1024 && hidden>256*1024,'the file fits, and hiding one more tool would not'); + assert.equal(p.edit('set_tool_visibility',{name:'example',visible:false},tools).error, + `Nothing was changed: with this change your tool presentation file would be ${hidden.toLocaleString('en-US')} bytes, ${(hidden-256*1024).toLocaleString('en-US')} over its limit of 262,144 (256 KiB). ` + +'Make room by restoring descriptions you no longer need with set_tool_description and "description": null. ' + +'Entries for tools no longer available to you can be removed only by whoever runs this host.'); + assert.equal(readFileSync(path,'utf8'),nearlyFull); + // Another writer changes the file after this edit read it. + writeFileSync(path,'{"version":1,"tools":{}}'); + const theirs='{"version":1,"tools":{"other":{"visible":false}}}'; + fs.writeFileSync=((...args:any[])=>{(originalWrite as any)(...args);if(typeof args[0]==='number')originalWrite(path,theirs);}) as typeof fs.writeFileSync; + syncBuiltinESMExports(); + assert.equal(hide(),'Nothing was changed: your tool presentation file changed while this edit was being made. Call this again; it applies to the file as it is now.'); + fs.writeFileSync=originalWrite;syncBuiltinESMExports(); + assert.equal(readFileSync(path,'utf8'),theirs); + // A failure no refusal names: the rename, the edit's last step. + fs.renameSync=(()=>{throw Object.assign(new Error('EXDEV: cross-device link not permitted, rename'),{code:'EXDEV'});}) as typeof fs.renameSync; + syncBuiltinESMExports(); + assert.equal(hide(),'Nothing was changed: the edit failed (EXDEV). Ask whoever runs this host to check your tool presentation file and its directory.'); + // One with no code is quoted, cut at 500 characters, since nobody wrote it for the agent. + fs.renameSync=(()=>{throw new Error('z'.repeat(600));}) as typeof fs.renameSync; + syncBuiltinESMExports(); + assert.equal(hide(),`Nothing was changed: the edit failed (${'z'.repeat(500)}… (600 characters)). Ask whoever runs this host to check your tool presentation file and its directory.`); + fs.renameSync=originalRename;syncBuiltinESMExports(); + assert.equal(readFileSync(path,'utf8'),theirs); + assert.deepEqual(fs.readdirSync(dir).sort(),['target.json','tools.json'],'no temporary copy or lock is left'); + // Cleanup after the rename can't undo the edit: a lock that won't go stays for the next edit to report, and this one succeeds, + // saying what the next edit will meet. An edit whose lock goes says nothing of it. + assert.equal('note' in (p.edit('set_tool_visibility',{name:'example',visible:true},tools).data as object),false); + const busy=Object.assign(new Error('EBUSY: resource busy or locked, unlink'),{code:'EBUSY'}); + const left="This edit couldn't remove its lock afterwards, the presentation file's name with .lock added (EBUSY). " + +'Until whoever runs this host removes it, set_tool_visibility and set_tool_description will be refused, saying an edit left its lock behind.'; + fs.unlinkSync=((target:fs.PathLike)=>{if(String(target).endsWith('.lock'))throw busy;return originalUnlink(target);}) as typeof fs.unlinkSync; + syncBuiltinESMExports(); + const done=p.edit('set_tool_visibility',{name:'example',visible:false},tools); + assert.equal(done.success,true); + assert.equal((done.data as {note?:string}).note,left); + fs.unlinkSync=originalUnlink;syncBuiltinESMExports(); + assert.equal(p.resolve(tools).advertised.some(t=>t.name==='example'),false,'the edit took effect'); + assert.match(hide()!,/^Nothing was changed: an earlier edit of your tool presentation file left its lock behind/); + fs.unlinkSync(path+'.lock'); + // A lock something else removed first isn't left: its unlink finds nothing, and the edit says nothing of it. + const gone=Object.assign(new Error('ENOENT: no such file or directory, unlink'),{code:'ENOENT'}); + fs.unlinkSync=((target:fs.PathLike)=>{if(String(target).endsWith('.lock')){originalUnlink(target);throw gone;}return originalUnlink(target);}) as typeof fs.unlinkSync; + syncBuiltinESMExports(); + const removed=p.edit('set_tool_visibility',{name:'example',visible:true},tools); + fs.unlinkSync=originalUnlink;syncBuiltinESMExports(); + assert.equal(removed.success,true); + assert.equal('note' in (removed.data as object),false); + // A refusal whose lock won't go says so too. + fs.unlinkSync=((target:fs.PathLike)=>{if(String(target).endsWith('.lock'))throw busy;return originalUnlink(target);}) as typeof fs.unlinkSync; + fs.renameSync=(()=>{throw Object.assign(new Error('EXDEV: cross-device link not permitted, rename'),{code:'EXDEV'});}) as typeof fs.renameSync; + syncBuiltinESMExports(); + assert.equal(hide(),`Nothing was changed: the edit failed (EXDEV). Ask whoever runs this host to check your tool presentation file and its directory. ${left}`); + fs.unlinkSync=originalUnlink;fs.renameSync=originalRename;syncBuiltinESMExports(); + fs.unlinkSync(path+'.lock'); + // A file the catalogue can't read says why: a link to nothing, a directory, a file this process can't open, a file too big to read. + fs.unlinkSync(path); + fs.symlinkSync(join(dir,'missing.json'),path); + const unusable=()=>p.resolve(tools).diagnostics[0]?.match(/^Your tool presentation file can't be used \((.*)\), so none of its settings apply/)?.[1]; + assert.equal(unusable(),"it's a symbolic link to a file that doesn't exist"); + fs.unlinkSync(path); + mkdirSync(path); + assert.equal(unusable(),"it isn't a regular file"); + fs.rmdirSync(path); + writeFileSync(path,'{"version":1,"tools":{}}'); + if (process.getuid?.() !== 0) { + chmodSync(path,0o000); + assert.equal(unusable(),"it couldn't be opened: EACCES"); + chmodSync(path,0o644); + } + // A path that runs through a file can't be opened, whoever runs the tests. + assert.deepEqual(new ToolPresentation({path:join(path,'tools.json'),cataloguePath:'board/tools.md'}).resolve(tools).diagnostics[0] + ?.match(/^Your tool presentation file can't be used \((.*)\), so none of its settings apply/)?.[1],"it couldn't be opened: ENOTDIR"); + writeFileSync(path,'x'.repeat(300_000)); + assert.equal(unusable(),"it's 300,000 bytes, over the 262,144 (256 KiB) this host reads"); + } finally {fs.writeFileSync=originalWrite;fs.renameSync=originalRename;fs.unlinkSync=originalUnlink;syncBuiltinESMExports();rmSync(dir,{recursive:true,force:true});} +}); + +test('the catalogue quotes a read error no code names, cut at 500 characters, since nobody wrote it for the agent',()=>{ + const dir=mkdtempSync(join(tmpdir(),'presentation-read-error-')); + const originalRead=fs.readSync; + try { + const path=join(dir,'tools.json'), p=new ToolPresentation({path,cataloguePath:'board/tools.md'}); + writeFileSync(path,'{"version":1,"tools":{}}'); + fs.readSync=(()=>{throw new Error('z'.repeat(600));}) as typeof fs.readSync; + syncBuiltinESMExports(); + const diagnostic=p.resolve(presentationTargets('example')).diagnostics[0]; + fs.readSync=originalRead;syncBuiltinESMExports(); + assert.equal(diagnostic?.match(/^Your tool presentation file can't be used \((.*)\), so none of its settings apply/)?.[1],`${'z'.repeat(500)}… (600 characters)`); + } finally {fs.readSync=originalRead;syncBuiltinESMExports();rmSync(dir,{recursive:true,force:true});} +}); + +test('a problem lists at most ten names, then how many more',()=>{ + const dir=mkdtempSync(join(tmpdir(),'presentation-lists-')); + try { + const path=join(dir,'tools.json'), defaults=join(dir,'defaults.json'); + const twelve=(prefix:string)=>Array.from({length:12},(_,i)=>`${prefix}${i}`); + const listed=(names:string[])=>`${names.slice(0,10).map(name=>`"${name}"`).join(', ')} and 2 more`; + const p=new ToolPresentation({path,cataloguePath:'board/tools.md',defaults:[{source:'MCPL server: big',path:defaults}]}); + const tools=[...presentationTargets('example',...twelve('big--t')),...presentationTools('board/tools.md')]; + // The agent's own call, with twelve arguments it doesn't take. + const extra=p.edit('set_tool_visibility',{name:'example',visible:false,...Object.fromEntries(twelve('x').map(key=>[key,1]))},tools).error!; + assert.ok(extra.startsWith(`Nothing was changed: ${listed(twelve('x'))} aren't arguments of set_tool_visibility. `),extra); + // The file, with twelve keys at its top level and twelve in one entry. + writeFileSync(path,JSON.stringify({version:1,tools:{example:Object.fromEntries(twelve('u').map(key=>[key,true]))},...Object.fromEntries(twelve('k').map(key=>[key,1]))})); + const file=p.resolve(tools).diagnostics[0]; + assert.ok(file.includes(`only "version" and "tools" belong at the top level, not ${listed(twelve('k'))}; `),file); + assert.ok(file.includes(`the entry for "example" has ${listed(twelve('u'))}, and an entry may have only "description" and "visible"`),file); + // A defaults file that sets "visible" for twelve tools. + rmSync(path); + writeFileSync(defaults,JSON.stringify({version:1,tools:Object.fromEntries(twelve('big--t').map(name=>[name,{visible:false}]))})); + const sources=new Map(twelve('big--t').map(name=>[name,'MCPL server: big'])); + const defaultsFile=p.resolve(tools,sources).diagnostics[0]; + assert.ok(defaultsFile.includes(`(it sets "visible" for ${listed(twelve('big--t'))}, and a defaults file may set only descriptions`),defaultsFile); + } finally {rmSync(dir,{recursive:true,force:true});} +});