fix(agents): close the failure modes a workflow-authoring session hit - #5330
Merged
Conversation
A session asked for a competitor-ad workflow and never got one. Twelve
rounds, no graph. Every wall it hit is fixed here.
**search_nodes scored characters.** The schema says `string[]`, and a
model that sent the bare string `"serpapi"` had it accepted: a string is
iterable, so the scorer walked it one character at a time and every node
holding an "s", an "e" or an "r" matched. The live registry answered with
322 results led by `CompareImages` at score 133 — noise the caller cannot
tell from a ranked answer. A lone string is one term now; anything else is
refused by name.
**list_nodes on an unknown namespace was a dead end.** `nodetool.web`
answered `{total: 0, namespaces: {}, nodes: []}`, which reads the same as a
namespace that exists and is empty. It now names the miss and hands back
the namespaces that do exist. A search that matches nothing likewise says
what the two default filters hide, instead of an empty list.
**find_model(...).ref was undefined.** Every caller wants one model, and
`results[0].ref` reads as if `results` and `ref` were alternatives. The
`undefined` went into a node's model property and surfaced much later as
"Property model requires a language_model to be selected" on a graph whose
model line looked right. The answer carries the best model's `ref` at the
top level, and the validator's hint says so.
**Interpolating a handle into a string had no way out.** "Pass it as the
property value itself" answers the one-handle case; a prompt built from
four upstream values has nowhere to go, and the session rewrote the same
template eight times. Both DSL cores now name `nodetool.text.Template` and
show the call.
**A misspelled pack export reached the guest.** `websearch` for `webSearch`
came back as QuickJS's "Could not find export", naming neither the module's
exports nor the near miss. Pack modules get the pre-flight capability
modules already had: `exportedNames` reads the resolved source, and the
refusal carries the list and the suggestion. `require()` and dynamic
`import()` are refused there too, instead of `ReferenceError: require is
not defined`.
**console.log(fn) killed the run.** The wrapper marshals an argument
property by property and a function's prototype chain leads back to itself,
so logging one — or a module namespace, which is what `import * as ns`
gives — blew the host stack, left handles alive and aborted the runtime with
`list_empty(&rt->gc_obj_list)`, after the guest's real work was done.
Arguments are formatted guest-side now, so only strings cross.
**Eight bullets for one problem.** `nodetool.workflows.validate` threw the
first eight issues of any severity, burying one error under seven "untyped
dynamic slot" info notes. It throws the errors, and counts the rest.
**Syntax errors pointed at the wrong line.** QuickJS compiles the whole
entry module and reported `import { x } = "…"` on line 3 as `expecting '('
at action:1:26`, with an excerpt showing a good line; the model rewrote line
1 twice. A body acorn also rejects is re-diagnosed with acorn's position.
The capability gate is fixed alongside: `suites` are file paths, so they do
not move when a case is added to a file already listed, and a description
edit demanded a contrived change to a generated field. A contract change
whose own covering suite is in the diff is answered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
A chat session asked for a competitor-ad workflow and never got one — twelve authoring rounds, no graph. Every wall it hit is a fixable product failure, and this closes all eight. In order of how much churn each one cost:
search_nodesscored characters. The schema saysstring[], and a model that sent the bare string"serpapi"had it accepted: a string is iterable, so the scorer walked it one character at a time and every node whose text held an "s", an "e" or an "r" matched. The live registry answered that query with 322 results led byCompareImagesat score 133 — noise a caller cannot tell from a ranked answer, and no signal that it passed the wrong shape. A lone string is one term now; anything else is refused by name. A search that matches nothing also says which of the two default-on filters could be hiding the answer, instead of returning an empty list.list_nodeson an unknown namespace was a dead end.nodetool.webanswered{total: 0, namespaces: {}, nodes: []}, which reads exactly like a namespace that exists and is empty. It now names the miss and hands back the namespaces that do exist, nearest first.find_model(...).refwasundefined. Every caller wants one model, andresults[0].refreads as ifresultsandrefwere alternatives.undefinedis not an error, so it went into a node's model property and surfaced much later as "Property model requires a language_model to be selected" on a graph whose model line looked right. The answer carries the best model'srefat the top level, and the validator's hint points at it.Interpolating a handle into a string had no way out. "Pass it as the property value itself" answers the one-handle case and nothing else; a prompt built from four upstream values has nowhere to go, and the session rewrote the same template eight times before giving up. Both DSL cores now name
nodetool.text.Templateand show the call.A misspelled pack export reached the guest.
websearchforwebSearchcame back as QuickJS'sCould not find export, naming neither the module's exports nor the near miss. Pack modules get the admission-time pre-flight capability modules already had:exportedNamesparses the resolved source, and the refusal carries the export list plus the suggestion. A module whose exports cannot be decided (export * from) is left alone — an unknowable list must not read as an empty one.require()and dynamicimport()are refused there too, instead ofReferenceError: require is not defined.console.log(fn)killed the run. The wrapper marshals an argument property by property and a function's prototype chain leads back to itself, so logging one — or a module namespace, which is whatimport * as nsgives — blew the host stack, left QuickJS handles alive, and aborted the runtime withlist_empty(&rt->gc_obj_list), after the guest's real work was done. Arguments are formatted guest-side now, so only strings cross: a function prints as[Function: name], a cycle as[Circular], everything else byte-identically to before.Eight bullets for one problem.
nodetool.workflows.validatethrew the first eight issues of any severity, burying one real error under seven "untyped dynamic slot" info notes — and a model reading eight bullets rewrites eight things.report.okis decided by the errors alone, so the throw lists the errors alone and counts the rest.Syntax errors pointed at the wrong line. QuickJS compiles the whole entry module, so
import { x } = "…"on line 3 arrived asexpecting '(' at action:1:26with an excerpt showing a perfectly good line; the model rewrote line 1 twice. A body acorn also rejects is re-diagnosed with acorn's position, so this can never override a failure the parser is happy with.One adjacent fix the diff needed: the capability gate's
suitesare file paths, and they do not move when a case is added to a file already listed — which is what covering a new contract usually looks like. A description edit therefore demanded a contrived change to a generated field, while writing the actual test satisfied nothing. A contract change whose own covering suite is in the diff is now answered.Verification
npm run test:affected— its plan is "everything", because apackages/sandbox-packs/file belongs to no workspace. Ran per-suite instead; every touched package and every direct dependent is green:agentsnode-sdkbase-nodeskernelcliexecutioncode-nodessandbox-compilernpm run typechecknpm run lint(exit 0; the warnings it prints are pre-existing and in files this diff does not touch)npm run dev:nodetool -- harness gate --base main— 6/6 selfchecks passednpm run build:sandbox-dsl:check— the hand-writtencore.jsedit leaves the 70 generated modules unchangedThree environment gaps in the container, all predating this branch and none touched by the diff.
base-nodesfirst failed 14 tests onNo WebGPU adapter available (Node/Dawn); extractingmesa-vulkan-driversand pointingVK_DRIVER_FILESat lavapipe — the recipe AGENTS.md documents, and what CI installs for that leg — took it to 33/33.@nodetool-ai/model3dhad nonode_moduleslink, sobuild:packagescould not finish until I symlinked it. Andpackages/protocolfails two suites onCannot find package 'ajv/dist/2020.js': the hoistedajvis v6 (a transitive dep) whilepackages/protocoldeclares^8.git stashreproduces all three.Every new check was inverted once and observed failing:
searchTerms→ the oldparams["query"] as string[]reads a lone string query as one term, not as its characters,refuses a query that is neither a string nor an arrayunknownPackExportsremovedrefuses an export the pack module does not have, with the listdynamicModuleAccessguard disabledrefuses require() and dynamic import(), naming the static formlists the errors and counts the restreparseSyntaxErrorremovedreports acorn's position, not QuickJS'sanswer.refremovedanswers the best model's ref at the top levelcore.jsrefuses concatenation, and the message says what to do instead(through the real QuickJS guest)The console fix was observed the other way round: before it,
console.log('fn:', function f(){})aborted the runtime withAborted(Assertion failed: list_empty(&rt->gc_obj_list), at: ../../vendor/quickjs-ng/quickjs.c,1954,JS_FreeRuntime).Agent capabilities
npm run capabilities:checkpasses (npm run capabilities:syncregenerated the three moved contract fingerprints)find_model,list_nodesandsearch_nodesre-declare their contract; each namespackages/agents/tests/capabilities-{models,nodes}.test.ts, and this diff adds the covering cases to those filesNew checks
leaves a module whose exports cannot be read alonepins that an undecidable export list means permission, andexportedNameshas its own suite asserting it finds the names in each export form🤖 Generated with Claude Code
https://claude.ai/code/session_01CsXu7kKHebJAeKPhyDZ18v