Repository navigation
Make a prompt's arguments one checked JSON object - #13
Merged
Merged
Conversation
A prompt declared a file under `input:`, and the Harness wrote the launch's `input_text` at that path before the run, or ended the run when it could not. The key, the `RunSetup` field, and `stage_input` are deleted, so a run starts on the store as the launcher left it. `prepare` has no failure left, so it returns `(Run, RunPacks)`. - `FileDecl` stays for `output:`. `Frontmatter::input` is deleted. - `prepare` takes no `run_id`, and `harness.rs` loses the two arms that ended a run on a `prepare` error. `of_run` is part of `refused`, its one caller. - An `input:` key now fails the parse as an unknown key. - The tests in `prepare-files.rs` and the round-trip test in `harness.rs` seed `paper.md` through `acquire_store`. `launch.rs` and the `run-prompt` example take their text from `args`. - Both `public-api.txt` listings lose `Frontmatter::input`, and the `runvil` listing loses `RunSetup::input_text`. - The three tests of a refused launch are removed with `input_undeclared` and `input_missing`. No test covers a frontmatter that has an `input:` key. Plan: /plan/2026-11-13-3-json-args.plan.md
A `var` table and an `argv` table are empty proxies over hidden data, so `#` gave 0, `pairs` gave nothing, and every conversion but `json.encode` read them as empty tables. A view now has a `__len`, `pairs` and `next` walk its data table, and `plain` gives `from_value` a copy of the data in place of each view. A value with no view inside converts as it did. - `read_only_proxy` sets `__len` only when its `index` is a table, and `var.rs` sets it for every `var` table. `view_data` reads a view with `unseal`, so `pairs`, `next`, and `tasks.fanout` use the definition of a view that `json.encode` uses. - `plain` returns the value itself when it holds no view. A table that holds a view becomes a fresh table that shares its other entries, and a copy keeps the list marker, so an empty list still converts to `[]`. - `plain` counts each value against a `WalkBudget` and fails with `the value is too large to convert`. It does not descend into a table already on its path or past `PLAIN_DEPTH_LIMIT`, so a table that contains itself fails in `from_value` with the same error as before. - Six sites call `plain`: the `args` of `tools.call`, a spawn's `item`, a `var` write, `render_item_value`, a bare global in prose, and a message record. `collection_members` takes a view's data table one level deep, so each member stays the author's own value. - `pairs` returns the view as its state and hands back each nested table as its view, so a write still crosses the guard. A `__pairs` metamethod still wins, and a view whose `__index` is a function walks as before. - `var.items[#var.items + 1]`, `table.insert`, and `table.remove` now work on a `var` list. - `pairs` over a sealed Pack global now yields its fields: the test in `prelude-tests.rs` expects `greet` where it expected no entry. - The run tests in `execute/tests/views.rs` pass `var` tables to a tool call, to `tasks.fanout`, to a spawn `item`, and to prose. No run test passes an `argv` table to them. Plan: /plan/2026-11-13-3-json-args.plan.md
A run took one argument string that nothing checked, and a prompt read it raw as `args` and parsed as `argv`. A run's arguments are now the text of one JSON object: the `args:` key declares its shape in a subset of JSON Schema, the first step of the run checks the object and ends the run with `args_invalid` at the first mismatch, and the read-only `args` global holds the checked object. A prompt with no `args:` key takes one optional string named `text`.
- `ArgsSchema` replaces `ArgsDecl`, `ArgDecl`, `ArgType`, and `ArgsDeclIter`. A serde visitor reads each node, so a refusal has the YAML path and the position of its node, and `as_json` gives the declaration as JSON Schema with `additionalProperties` written on each object.
- `accept` in `args_check.rs` fills the default of each missing field, makes a whole number under an `integer` node an integer, and reports the first mismatch as `ArgsInvalid` with the facts `Reason` and `Name` and no location. The parser checks each declared default with the same `check`.
- `ChainState::root` calls `accept_args` and keeps a refusal in `args_refusal`, which `check_start` returns after the version gate, so `Run::new` still returns a run for any text.
- A chain holds its `args` as an `Arc<serde_json::Value>`. `derive_argv`, the `argv` substitution root, and `nil_argv` are deleted, and a placeholder reads a field by a key path, as in `{{ args.text }}`.
- The guard on `_G` serves `args` and refuses an assignment to it. `argv.rs` is renamed `args.rs`, and `RESERVED_NAMES` has 62 entries.
- `call` and `tasks.spawn` take `args` as a table of named fields, which `to_json` converts, or as a string, which `with_args` makes the one field `text`. A list is refused with `got a list`. Nothing checks the arguments of a chain against the declaration.
- `TaskStarted` has `args` as an `Option<serde_json::Value>` in place of `input`.
- A required field with a default takes the default, and a `null` for a field that is not required counts as the field left out.
- The setup of a run has `{}` as its default `args`.
- `greet` declares a required `name`, `research-person` and `analyst-example` declare a required `text`, and `echo` returns `args.text`.
- Both `public-api.txt` listings change, and neither names `serde_yaml_ng` after the change.
- `Chain::blocked` gives `None` for a chain that holds the answer it was parked on, unless it waits for its slots. `tasks.status` read `blocked` as `chat` for a task whose reply had arrived and whose round was already in `turns`.
- The `var` guard refuses a key with no place in JSON data at the assigning line: `key_refusal` takes a string, or under the root the index of an item the table holds or of the next one. Such a key passed the write and failed the read-back of `var` with a raw `deserialize error`, or as an internal fault for an integer key of the root.
- A table that the conversion of a `var` write refuses raises `must be JSON data:` with the reason of the conversion. A table with list items and named keys still loses the named keys in a `var` write.
- No test names `argv`, the string form of `args`, or a spawn option `input`. A spawn option that `tasks.spawn` does not know is ignored, as before, and no test covers that.
Plan: /plan/2026-11-13-3-json-args.plan.md
A `var` write of a table with list items and named keys stored the list items alone, and `tasks.spawn` ignored an option that it did not take. A `var` write now converts its value by the rules of `json.encode` and refuses a write that would leave its table neither a list nor an object, and `tasks.spawn`, `tasks.join_any`, and `tasks.join` refuse an unknown key of `opts`.
- `to_stored_json` runs the `Encoder` of `json.encode` with the target as the root of each path, so `var.t = { 'a', name = 'x' }` raises `var.t mixes list items and named keys`. It sets `keeps_floats`, so a float with no fractional part reads back as a float.
- `write_refusal` replaces `key_refusal`. It also refuses a named field in a list, a list item in a table that holds named fields, and the removal of an item that is not the last of its list.
- A table that contains itself is refused as `contains itself`, and NaN as `is not a finite number`. The conversion of the Lua binding gave its own text for the first and stored the second.
- `check_value` and `CHECK_DEPTH_LIMIT` are deleted from `walk_budget.rs`. The `Encoder` spends the same `WalkBudget`, so a value too large to walk still raises `is too large to store`.
- `check_keys` in `__impl_tasks.lua` walks `opts` with the Engine's `pairs`, so the first unknown key in sorted order is the one named, and a table read from `var` is checked as its data. The refusal of a spawn names the key and lists `args`, `item`, and `index`.
- `ALLOWLIST` in `build-xtask` loses `serde_yaml_ng`, which neither public API listing names.
- No test covers a `var` table nested more than 127 levels deep, which the `Encoder` refuses.
Plan: /plan/2026-11-13-3-json-args.plan.md
|
Important Review skippedToo many files! This PR contains 167 files, which is 17 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to Team to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
 |
 [High impact] The PR changes argument handling across prompt parsing, run startup, Lua sections, child chains, and Host-facing APIs. The PR appears safe to merge, with a non-blocking fix recommended for cyclic views sent to tools. Findings
Fix with agent prompt### Issue 1
crates/runvil-internal/lua/src/proxy.rs:211-212
`PlainWalk::value` leaves a view unchanged when its hidden table is already on the current path. For example, `local data = {}; local view = setmetatable({}, { __index = data, __metatable = 'view' }); data.self = view` passed to `tools.call` becomes `{"self":{}}`. The copied field still holds the empty stand-in, which `from_value` reads as `{}`.
Refuse the cycle instead of passing the view through, so the tool does not receive shortened data without an error.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+211
to
+212
| if self.path.len() >= PLAIN_DEPTH_LIMIT || self.path.contains(&pointer) { | ||
| return Ok(None); |
There was a problem hiding this comment.
PlainWalk::value leaves a view unchanged when its hidden table is already on the current path. For example, local data = {}; local view = setmetatable({}, { __index = data, __metatable = 'view' }); data.self = view passed to tools.call becomes {"self":{}}. The copied field still holds the empty stand-in, which from_value reads as {}.
Refuse the cycle instead of passing the view through, so the tool does not receive shortened data without an error.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/runvil-internal/lua/src/proxy.rs
Line: 211-212
Comment:
**Cycles become empty objects**
`PlainWalk::value` leaves a view unchanged when its hidden table is already on the current path. For example, `local data = {}; local view = setmetatable({}, { __index = data, __metatable = 'view' }); data.self = view` passed to `tools.call` becomes `{"self":{}}`. The copied field still holds the empty stand-in, which `from_value` reads as `{}`.
Refuse the cycle instead of passing the view through, so the tool does not receive shortened data without an error.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
This was referenced Nov 14, 2026
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.
Summary
A caller passes a prompt one JSON object. The frontmatter key
args:declares the object's shape in a subset of JSON Schema, the run's first step checks the object against it, and the read-only Lua globalargsholds the checked object. A prompt's arguments then have the form of a tool's arguments, so a Host can publish any prompt as a tool.The input file, the raw argument string, and the
argvglobal are gone. Nothing recognizes a removed form: an old prompt fails by the ordinary rules.{"query":"papers"}returnspapers, limit 3.{"query":5}ends the run at its first step:argument `query` must be a string, got integer.What changed
Four commits, in order.
6ef23a8eRemove the input file. The frontmatter has noinput:key,RunSetuphas noinput_text, and the Harness stages nothing.output:is unchanged.8b7ad5f9A view behaves like its table. Avartable and every read-only table are empty stand-ins over hidden data.#,pairs, andnextnow read the data, and a view handed totools.call,tasks.fanout, a spawn'sitem, orvararrives as its data.var.items[#var.items + 1] = xappends.3d87c936The args rule.args:takes the keywordstype,properties,required,additionalProperties,items,enum,description, anddefault. Any other keyword fails the parse, with the YAML path and position of its node.additionalProperties: true. A missing field takes itsdefault, at every depth. Anullfor a field that is not required counts as the field left out.args:key takes one optional string namedtext.args_invalid(705), with no location and no event.call(heading, args)andtasks.spawn(target, { args = ... })take a table of named fields, or a string that the chain reads asargs.text.tasks.status, and thevarguard refuses a key with no place in JSON data.0d938885Refuse what would be lost. Avarwrite converts by the rules ofjson.encode, sovar.t = { 'a', name = 'x' }raises where it stored["a"]and droppedname.tasks.spawn,tasks.join_any, andtasks.joinrefuse an unknown key ofopts. The API allowlist losesserde_yaml_ng.What a Host changes
RunSetup.argsis the text of one JSON object, and its default is{}.RunSetup.input_textis gone.runvil::prompt::ArgsSchemastands whereArgsDecl,ArgDecl,ArgType, andArgsDeclIterstood.ArgsSchema::as_json()gives the declaration as plain JSON Schema, ready to publish as a tool's input schema.Frontmatter::input()is gone.ErrorCode::ArgsInvalidis a new code.Event::TaskStartedhasargs, a JSON value, where it hadinput, a string.Both
public-api.txtlistings are blessed.Verification
Run locally on Windows on the final tree, all passing:
cargo fmt --all --checkcargo clippy --workspace --all-targets --all-featureswithCARGO_BUILD_WARNINGS=denycargo nextest run --locked --workspace --all-features: 3,787 passedcargo docon the workspace, onrunvil-engine, onrunvil, and onrunvil-runtimewith private items, withRUSTDOCFLAGS="-D warnings"cargo +nightly-2026-09-05 xtask api --checkcargo dylint --all -- --locked --workspace --all-targets --all-featurescargo test -p build-xtasknode --test tools/docs-claims.mjscargo deny checkcargo run -p runvil --example run-promptThe plan and what the build found are in
plan/2026-11-13-3-json-args.plan.md.Related
0d938885and show theargsschema runvil-workshop#21 moves Workshop's pin to the head of this pull request.varshape rules and the refusal of an unknown option runvil-docs#5 and State the shape rules ofvarand the Run window'sargsrows runvil-design#3 state the rules of the last commit in the guide and the design pages.nameargument in thegreetsample runvil#216 brings thegreetsample of the README in line.