Honor declared input and output files in the harness - #88
Merged
Merged
Conversation
Author code could replace the `_G` metatable and so remove the `argv` freeze or the read-only `prose` guard. Now `globals::install` puts one guard metatable on `_G` at VM construction, and `argv.rs` and `prose.rs` only record their state in the guard's slot table through `freeze_argv` and `install_prose`. The `setmetatable` and `getmetatable` globals become replacements from `__impl_globals.lua` that keep an author metatable on `_G` behind the guard. - `harden` installs the guard first, because the guard chunk captures `rawequal`, `rawget`, and `rawset` before hardening removes them, and every later chunk, the `table.concat` wrapper included, must capture the replacements. - The guard carries `__metatable`, and the replacements compare the target with `rawequal`, so only `_G` gets special handling. For every other value they call the base functions and restore the function name in argument errors. - The `lua/AGENTS.md` rules now forbid host code to call `set_metatable` on the globals table or to raw-set `argv` outside H1 or `prose`. - The guard serves `argv` and `prose` before it reads the author metatable. In the H1 pass `argv` is a plain global that the author metatable never sees, so a repair lands in `_G` under a strict or write-hooking metatable. - The guard reads the author `__index` and `__newindex` at each access. It copies the other author fields onto itself only when `setmetatable(_G, mt)` runs. - `prose` now refuses assignment before the first block installs its render, so the shared library cannot set a raw `prose` global. Until then it reads nil. - `globals-tests.rs` checks each `setmetatable(_G, ...)` form against the guard and compares other values with stock Lua through `stock_eval`. The engine `global_metatable` suite runs author metatables through the shared replay, H1, later fences, and later sections.
A tool alias or model role label installs as a section VM global of its own name, so a key that names a host global or a sandbox Lua global replaces that global. `RESERVED_NAMES` in `globals.rs` lists the 60 reserved names with their `Reserved` kind, and the parser now refuses a reserved key under `tools` or `models` and a name declared under both maps. The prelude install checks capability globals against the same list through `reserved_name`, in place of `RESERVED_GLOBALS`.
- `deserialize_contract_map` takes a `ContractKeys` value in place of the `what` and `reserved` arguments. The old `reserved` key is now `deferred`, and `installs_global` selects the reserved-name check, which `args` turns off because arg names are `argv` fields.
- The parser reads the list through `promptforge_lua::reserved_name`, so the parse refusal and the prelude refusal use one list. The lua `AGENTS.md` requires a new host global to go on `RESERVED_NAMES` in the same change, and the engine `lua::tests::globals` compares the list against a set-up VM in both directions.
- A reserved key fails as `Frontmatter` at the key's own line and column. The both-maps refusal from `check_distinct_aliases` runs after the frontmatter parse, names the first shared name in sorted order, and has no line or column.
- `_G` and `_VERSION` fail the alias grammar before the reserved check. `global` is on the list as a keyword, though the vendored Lua lexer reads it as contextual.
- The prelude collision text changes from `which is reserved for {holder}` to `which is reserved as {kind}`, and a prelude global now also may not take a Lua keyword.
- An alias that matches a capability prelude global, such as `input`, still parses. The run then fails at the first section VM setup, before any effect, and a new test in `prepare-input.rs` pins this.
- The parser `tests.rs` covers one sample of each kind in both maps, every `RESERVED_NAMES` entry in both maps, near-miss names such as `Store` and `my_argv`, reserved arg names, and the both-maps refusal. The facade `prompt.md` adds a doctest for a `store` alias.
The rooted host backend resolved each path through its links before acting, so an operation on a symlink landed on the link's target. Removing a link to a file inside the root deleted that file, and checking, describing, creating over, renaming, or removing a link to a target outside the root failed as an escape. Operations on a path itself now check only the parent directory against the root and leave the final component unresolved, so they act on the link itself. Operations on file contents still follow links and still refuse a link that leads outside the root. On Windows a directory link or junction is removed as a directory entry, because removing it as a file fails. - `contain_no_follow` runs the parent through `contain`, so a link in an earlier component still resolves and is still denied when it leaves the root, then appends the final component unresolved. The mounted root resolves to itself. - `resolve_no_follow` now serves `remove`, `exists`, `stat`, `mkdir`, and both ends of `rename`, and in identity mode it returns the host path exactly as `resolve` does. The module doc states which operations take which resolution. - `is_dir_link` is true only on Windows, for a directory symlink or junction, and `remove` sends such an entry to `fs::remove_dir` whatever `recursive` says. Off Windows it is always false, and `fs::remove_file` removes any link. - `stat` in rooted mode describes a link itself as `FileType::Symlink`, where it used to describe an in-root target or refuse an outside one. `exists` is true for any present link, and `mkdir` over one returns `AlreadyExists`. - `make_file_link` returns false only for raw OS error 1314, `ERROR_PRIVILEGE_NOT_HELD`, and prints a skip notice, so the file-link tests return early and pass on a Windows host without the symlink privilege. The directory-link tests assert that `make_dir_link` succeeded instead of skipping. - `removing_a_dangling_link_succeeds` pins behavior the old resolution already had, since a dangling final component was never canonicalized. - `resolve` still serves `read`, `read_range`, `write`, `append`, `list`, `glob`, and `copy`, so contents operations follow links, and the escape denial text is unchanged. Design: new pure-function @ crates/promptforge-internal/vfs/src/host.rs::is_dir_link deps: fs::Metadata Repairs: remove deletes a final-component link, never its target @ crates/promptforge-internal/vfs/src/host.rs::HostAccess::remove - removing a link to an in-root file or directory acted on the target instead of the link Repairs: path operations address a final-component link as a link @ crates/promptforge-internal/vfs/src/host.rs::HostAccess::resolve_no_follow - exists, stat, mkdir, rename, and remove on a link to an outside target failed with the escape denial Plan: vibe/2026-09-28-4-internal-crates-critical-fixes.md
Author Lua that caught errors in a loop could keep a cancelled run alive forever. Cancellation aborts a running block by raising an error from the instruction hook, and the shim's protected calls caught that error and returned it to the author like any other failure, so a loop around a protected call started the work again each time. The shim now checks the run's cancel flag whenever a protected call fails and raises the failure again while the flag is set, so the abort unwinds to the block guard and the run ends as interrupted. The same check covers the shim's own protected calls around a local tool handler and a context compactor. - `install_shim_prelude` takes the VM's `InstructionBudget` and hands the shim chunk a new last argument, `cancel_requested`, a Lua function returning `budget.is_cancelled()`. The closure holds a clone of the whole budget, which shares its cancel flag and hook counter with the VM; `install_coro_shims` passes its `instruction_budget` field and keeps its signature. - `xpcall_outcome` wraps both branches of `protected_xcall`. With a function message handler, the value raised under cancellation is whatever the handler returned, not the raw failure the other sites raise. - `pcall_outcome` raises the raw failure at level 0 when `cancel_requested()` is true, instead of returning false and the normalized failure. The check reads only the flag, so any failure caught after cancellation is set is raised again, an author's own `error` call included. - `run_local_tool` still calls `leave_local_handler()` first, then under cancellation raises the handler's failure at once, with no `local_tool_done` yield. - `compact` raises the compactor's raw failure under cancellation instead of the normalized error table. A compactor that returns still raises the deferred-replacement error, cancelled or not. - `a_cancelled_run_unwinds_through_an_author_pcall_loop` and `a_cancelled_run_unwinds_through_an_author_xpcall_loop` start a looping block on a `shim_vm` whose cancel handle is set before the block runs, and expect `Error::Interrupted`. `a_pcall_failure_without_a_cancel_flag_still_returns_false_and_the_error` pins the flag-clear path. - `cancel_during_a_looping_local_tool_handler_returns_promptly` and `cancel_during_a_looping_compactor_returns_promptly` cancel after 100 ms on a two-worker runtime and require `crate::Error::Interrupted` within 5 seconds. Their bodies are near-identical past the prompt setup. - `local_tool_done` is not asserted on under cancellation: the two engine tests check only elapsed time and the result, so neither pins the skipped yield or the raw compactor failure. Design: new shared-mutable-state @ crates/promptforge-internal/lua/src/coro.rs::install_shim_prelude deps: Arc<AtomicU32>,InstructionBudget,Lua,usize Design: extends constructor-injection @ crates/promptforge-internal/lua/src/__impl_coro.lua Design: new clone-block @ crates/promptforge-internal/engine/src/execute/tests/models_loop_compactors.rs::cancel_during_a_looping_compactor_returns_promptly Repairs: the shim pcall re-raises a failure caught under cancellation @ crates/promptforge-internal/lua/src/__impl_coro.lua::pcall_outcome - an author loop around pcall kept a cancelled run spinning forever Repairs: the shim xpcall re-raises a failure caught under cancellation @ crates/promptforge-internal/lua/src/__impl_coro.lua::protected_xcall - an author loop around xpcall with a message handler kept a cancelled run spinning forever Plan: vibe/2026-09-28-4-internal-crates-critical-fixes.md
Reports from a chain on the walk now carry the name of the section the chain last entered, recorded at entry, instead of a name looked up from the walk position. The lookup indexed past the end when a walk ran off its last section with a model task still live, and between sections it named the next section rather than the one just finished. The scheduler's call-nesting stack is gone: nothing decided from it, and its debug check fired when two tasks' call children finished out of nesting order. Releasing a task slot now saturates, and the run-limit defaults are evaluated at compile time. - `entered` is a new owned field on the chain, set when the walk builds a section's frame and initialized at both chain construction sites to the slice's first section name, or the prompt title for an empty slice. Outside the H1 pass, `section_name` returns it instead of indexing the slice at `index`. - `stack` is deleted from the scheduler with its push, pops, and clear; call dispatch only enqueues the child, and the recursion cap keeps reading `call_depth`, which carries depth across calls and spawns alike. - `section_name` now reports the section just left for a chain between sections or past its slice's last section, so a notice queued at the walk's end or between two sections lands under that section. - `saturating_sub` keeps a task slot release from wrapping the counter in release builds; the `debug_assert!` above it stays, so debug builds still catch an over-release. - `RunLimits::new` and its test wrap each `nz_*` default in a `const` block, so a bad literal fails at compile time. - `a_task_that_ends_while_its_owner_is_between_sections_reports_under_the_section_just_ended` drives the serial driver rather than the mock gateway, and its timing depends on that driver's fixed answer order. Repairs: a chain's reports name its section @ crates/promptforge-internal/engine/src/execute/scheduler.rs::Chain::section_name - a walk that ran off its last section with a live model task panicked with an index out of bounds when the abandoned task's notice was queued Repairs: a notice names the section its owner last entered @ crates/promptforge-internal/engine/src/execute/scheduler.rs::Chain::section_name - a task that ended while its owner was between sections reported under the next section instead of the one just ended Repairs: call children finish in any order @ crates/promptforge-internal/engine/src/execute/scheduler/chain.rs - a debug build panicked when one task's call child finished while another task's later-dispatched call child was still running Plan: vibe/2026-09-28-4-internal-crates-critical-fixes.md
The model client stops re-exporting the streaming delta and the call metrics types, which have their canonical home in the shared types crate, and its own modules import them from there. An attribute with no effect on a crate-private type is dropped. Docs and comments across the internal crates now describe what each crate holds and does, and comments that narrated history or cited audit tags are restated as present-tense constraints or deleted. Two groups of flat sibling files in the engine move into their parent modules' directories, so those parents declare them without path overrides. Nothing changes at run time apart from the names of test temporary directories. - `StreamDelta` - The `client` module no longer re-exports it. `client/read.rs` and `client/stream.rs` import it from `promptforge_types::wire` directly. - `promptforge_types::metrics` - The model client's crate root no longer re-exports `CallMetrics`, `ClientTiming`, `LlamaTimings`, `Usage`, or `VllmMetrics`. The crate docs name them in this module instead. - `NormalizedTurn` - Drops `#[non_exhaustive]`, which does nothing on a crate-private struct. - `engine/src/execute/run/` - The run effect module, its tests, and the run tests move into this directory. `run.rs` declares `mod effect` and `mod tests` without `#[path]`, and `effect.rs` names its tests `effect-tests.rs`. - `engine/src/test_support/recording/` - The forward module, its tests, and the observation module move into this directory. `recording.rs` drops both `#[path]` attributes, and `forward.rs` names its tests `forward-tests.rs`. - `TempDir` - The engine VFS suite and the facade prepare suite create temp directories prefixed `promptforge-engine-vfs-` and `promptforge-prepare-` instead of `promptforge-api-`. - `RequirementsUnmet` - The docs on the error variant and on `RunResult` now list every cause: a missing required capability, a missing host service, a capability conflict, an unmet model requirement, or a failed H1 hard gate. - `ToolCall` - Its doc says the wire decoder stores the arguments as a decoded JSON object and fails the turn when they are missing, not a string, not valid JSON, or not an object. The old text claimed a fallback to a string value. - `AGENTS.md` - The root rule now says `promptforge-types` holds the shared vocabulary together with host-support code, replacing the claim that the types crate holds wire vocabulary only and never code. - `crates/promptforge-internal/vfs/README.md` - New README covering what the crate holds, its std-only dependency rule, and the host backend's two resolutions: path operations act on a final-component link as a link, and content operations follow links under the containment check. - `TaskNote` - Its doc now marks the variant as reserved and not yet produced. - `## Minimum Rust Version` - The engine README drops this section, and its license line no longer links to `LICENSE`. Design: removes shim @ crates/promptforge-internal/model-client/src/client.rs::StreamDelta boundary: pub Design: removes shim @ crates/promptforge-internal/model-client/src/lib.rs boundary: pub Plan: vibe/2026-09-28-4-internal-crates-critical-fixes.md
Plan: vibe/2026-09-28-4-internal-crates-critical-fixes.md
A prompt can declare an `input:` file and an `output:` file, but the harness did not write the first or read the second, and a client could not give a session its own filesystem. The harness now writes `LaunchRequest::input_text` at the declared input path before each run and keeps what the completed run wrote at the declared output path for `Session::output_text`. `Harness::launch_with` takes `LaunchOptions`, whose `vfs` is the filesystem that every run of the session uses. - `VfsRef::acquire_store` is the one addition to the engine API: it is `acquire` followed by the store view, in a new scope. The harness writes and reads the declared files through it, so the store path rules apply to the frontmatter paths and the store can be at any root. `public-api.txt` gets one line for it. - `stage_declared_input` runs after parse and before capability activation, on the blocking pool through `spawn_blocking_launch`. A refusal closes the run row with kind `Input` and returns `PrepareError::Input`. - `InputFileError` has three cases: `Undeclared` for text when the prompt declares no input file, `Missing` for a declared file that the launch does not supply and the store does not hold, and `Store` when the store refuses. - `LaunchOptions::vfs` is shared by every run of the session, relaunches included. `None` keeps a fresh memory store for each run. `harness::vfs` re-exports `VfsRef` and `VfsError` because harness signatures use them, so `harness` now depends on `promptforge`. - The harness reads the output only when a run ends `Completed`, and it keeps the result before the session reports `Closed`. `OutputError` is `Unfinished`, `Undeclared`, `Missing`, or `Store`. A missing output does not fail the run. - New tests: `prepare-files.rs` pins the staging, the three refusals, and a host-seeded input. `session-files.rs` pins the round trip, a host filesystem with the store at `/store` and an overlay, and each `OutputError` case. `launch.rs` and four `acquire_store` tests in `detail.rs` cover the new API. - `agents.rs` still calls `launch`, so Workshop sessions keep a fresh memory store for each run.
This branch was successfully deployed
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
1 participant
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.
Stacked on #87. This branch sits on top of #87's seven commits, so the commit list and diff here include them until #87 merges. The one new commit is a05d5cb, "Honor declared input and output files in the harness". Review that commit on its own, and merge this after #87.
A prompt can declare an
input:file and anoutput:file in its frontmatter, but the harness never wrote the first or read the second, and a client had no way to give a session its own filesystem. This makes the harness honor both declarations and lets a host hand a session anyVfsRefit builds: overlays, host mounts, seeded files, its own store backend, a policy, or an op sink. The engine gains one method, and the harness gains one request field, one options struct, one launch method, and an output accessor.Changes
Public API
VfsRef::acquire_store(origin)in the engine: a store view in a scope of its own, so a host seeds and reads store files by the names the prompt uses, wherever the handle mounts the store.public-api.txtgains that one line.LaunchRequest::input_text: Option<String>: the text staged at the declared input path before each run. It's#[serde(default)]and skipped when absent, so the serialized request is unchanged without it.LaunchOptions { vfs: Option<VfsRef> }andHarness::launch_with(request, options): the filesystem every run of the session works in, relaunches included.launchdelegates with the default options, which keeps today's fresh memory store per run.Session::output_text() -> Result<String, OutputError>: what the completed run left at the declared output path.OutputErrorisUnfinished,Undeclared,Missing { path }, orStore { path, source }.harness::vfsre-exportsVfsRefandVfsError, because harness signatures name them, so theharnesscrate now depends onpromptforge.Behavior
path: ../xcan't escape the store.input:, or when the prompt declares one that the launch doesn't supply and the store doesn't already hold. The row closes under theInputkind and the client gets aRunFailedreport. A host that seeds the file into its own filesystem satisfies the declaration.Completedand before the session reportsClosed, so awaitingClosedand then callingoutput_text()doesn't race. A missing output is reported, not treated as a run failure.Not in this PR
launch, so its sessions keep a fresh memory store per run, andHostSnapshot::workspace_rootsstill mounts nothing.Testing
acquire_store(a non-root store, traversal refused, no store isUnsupported, a scope of its own); 7 runner tests covering staging, each of the three refusals, a host-seeded input, and the output path; 6 session tests covering the round trip, a host filesystem with the store at/storeplus an overlay, a host-seeded store, eachOutputErrorcase, and the refusal arriving asRunFailed; and 3 facade tests.-D warningsand rustfmt are clean, the docs build with warnings denied (the vfs private items included),cargo xtask tidyandcargo hakari verifypass, andcargo xtask api --checkon the pinned nightly shows only the newacquire_storeline.headlessfeature. Its full build fails locally in the UI bundling step, where esbuild can't resolve@workshop/platform/*. That's in the local Node workspace, not this change.