Workshop desktop shell: title bar, workspace files, editor, and chat polish - #5
Closed
vinniefalco wants to merge 604 commits into
Closed
Workshop desktop shell: title bar, workspace files, editor, and chat polish#5vinniefalco wants to merge 604 commits into
vinniefalco wants to merge 604 commits into
Conversation
Four callback sites in `install_live_tools` repeated the same sequence for keeping the first resolver error. That sequence is now `BindingState::record_error`, which stores the error only when `callback_error` is still empty, so the first concrete `Error` keeps its typed cause. - The four call sites behave as before; no tests change.
Doc comments across the crate drifted from the code they describe. This pass corrects stale references and wording, adds missing `# Errors` contracts and missing doc comments on crate-internal items, and fixes the user guide to say the generic `"done"` result applies only when no model reply exists. - `build_sections` now documents `Error::ParseStructured`, `Error::LuaCompile`, `Error::Lua`, and `Error::Internal`; `SharedTools::new` documents `ToolRegistryError::InvalidWireName`; `StoreRef::str_replace` documents `StoreError::InvalidAnchor`. - New doc comments cover `BindingState`, `validate_alias`, `scalar_return`, `from_live_binding`, and the `binding` accessors. - No executable code changes; one stale test comment in `parser/tests.rs` is corrected.
Several production paths carried the same logic in two or more places. The repeated blocks move into shared helpers, including `build_request_body` in `transport.rs`, `messages_array_mut` in `dispatch.rs`, `scan_top_level` in `codec.rs`, `consume_peel` in `content.rs`, `render_signature` in `guide.rs`, `read_store_bounded` and the `install_reported_store_fn!` macro in `host.rs`, `compile_chunk` in `program.rs`, `enrich_sys_field` in `sys.rs`, `build_heading_blocks` in `build.rs`, `with_read_range` in `store.rs`, and `render_scalar` in `subst.rs`. - `error.rs` gains the `BoxedSource` alias, and every boxed error source field uses it. - The fanout scheduling loop splits into `fill_window` and `handle_joined_arm`, with the abort path shared. - No behavior change is intended; the commit touches production files only and no tests.
Several execution paths could misorder or misreport failures. Live H1 prose is now substituted and skipped when empty before a model is required, `inject_host` failures fire the teardown observation pair, a caught resolver callback error fails its own block instead of surfacing later, and cancelling an in-flight infer no longer reports `MODEL_TURN_FAILED`. - `guarded_var` now rebuilds every nested table as its own proxy over hidden data, so later writes cross the assigning-line validation; `var_to_json` materializes the proxies before conversion. - The local tools `schemas` and `contains` reads fail closed on a poisoned lock, `newlines_before` returns `Error::Internal` instead of panicking on an invalid offset, and `compile_glob` debug-asserts its validated-grammar precondition. - The model option decoders take an entry-point label, so a `models.default` failure names `models.default`, and the no-tools violation message now reads "model inference received tool calls but no tools were advertised". - New tests pin each change, including `cancelled_nested_infer_does_not_report_model_turn_failed` through the new `resp_delayed_text` gateway reply and `guide_sorts_parameter_names_and_preserves_required_markers` for sorted guide parameters.
Several tests asserted less than their names claim, through reflexive comparisons, loose substring checks, or missing cases. Assertions now pin exact values, full observation sequences, per-case error messages, and boundary lengths, and two tests are renamed to match what they verify. - `logs_are_correlated_and_ordered_across_chunks` asserts the exact ordered record sequence, and the glob test checks the compiled and one-shot matchers against independent expectations instead of against each other. - New pins cover the 64 and 65 character `validate_alias` boundary, the 1024 and 1025 byte store path boundary with `PathReason::TooLong`, and Display opacity for backend and tool error sources. - All changes sit inside test modules; production code is untouched.
Test modules repeated the same gateway client construction, run options, and prompt fixtures many times. The repeated blocks move into shared helpers such as `gateway_client`, `gatewayed_with_debug`, `client_for`, `add_local_md`, `parse_turn`, `resolve_shared`, and `assert_invalid_paths`, and the call sites adopt them. - `gatewayed(addr)` and `gateway_client(addr)` in `execute/tests/mod.rs` are now the single construction point for scripted gateway runs. - No assertions change; the suite loses about 750 net lines.
The workspace moves `h2` and `indicatif` to newer releases. `Cargo.toml` raises the `indicatif` requirement to 0.18, and `Cargo.lock` records `h2` 0.4.18 and `indicatif` 0.18.6. - `indicatif` 0.18.6 replaces `number_prefix` with `unit-prefix` and pulls in `console` 0.16.4; `Cargo.lock` drops `windows-sys` 0.59.0. - No Rust source files change.
The execute subtree gains one ambient state object so a new run-scoped concern becomes a field instead of a new parameter. `RunContext` starts with one field, `prompt: Arc<Prompt>`, built once in `run` and passed as parameter one to `execute_live_h1` and `run_sections`. - `engine.rs` renames its `RunFrame` bindings and parameters from `ctx` to `frame`, so `ctx` names the `RunContext` alone. - `RunContext::new` clones the prompt into an `Arc`; a unit test pins that clones share the one allocation. - The struct carries `prompt` only; the module docs state the invariant that later run-scoped concerns accrete here as fields.
One `GuardNonce` minted in `RunContext::new` now serves every `untrusted::wrap` in the run, so identical untrusted content produces a byte-identical envelope and KV-cache prefixes stay shared across tool-loop rounds and fanout arms. The nonce threads from `RunContext` through `RunFrame`, `ControlContext`, `FanoutContext`, and `SectionProgress` into `SectionVm::new`, whose `untrusted` global captures an owned clone. - `GuardNonce` becomes `pub(crate)` and gains `Clone` and `Debug`; `wrap` now takes the run's `nonce` instead of minting one per call. - `install_untrusted` clones the nonce because mlua `create_function` requires `Fn + Send + 'static`, so no borrow crosses the install. - The `encode` escaping of every literal `<` is unchanged; only the nonce's source moves. - The freshness test inverts to `one_nonce_wraps_every_envelope_with_identical_tags`, and the new `untrusted_nonce_differs_across_runs` pins that two runs of one prompt wrap the same tool output under different nonces.
`execute`, `jump`, `fanout`, and `list_from_section` install into the H1 VM as stubs that fail with a message naming the cause, because H1 runs before any section exists and the real control globals can never operate there. `SectionVm::install_h1_control_stubs` installs the four stubs, and `execute_live_h1` calls it right after `install_host_apis`.
- Each stub raises `{name} is only available in sections (## headings); H1 runs before sections exist` for any arguments.
- The stale `list_from_section_is_absent_on_the_h1` test is replaced by one clear-error test per global.
- The recorded-jump arm in the live H1 block walk stays defensive; the stubs raise before anything is recorded.
The per-section state of a walk entry - the VM, the `sys` JSON, `var`, the reply, the conversation, the counts, the completion options, the write scope, the depth, and the effective reporting handles - collects into the new `SectionContext`, with construction, the block walk, and teardown as methods. `run_one_section` becomes one construct-run-teardown cycle over the frame. - `run_one_section_impl` stays a free function taking split borrows of the frame's fields, because the unconverted H1 and fanout drivers still call it from locals. - `SectionProgress` dissolves: `run_prose_inference` takes its former fields as plain parameters, and the test-only `silent_progress` helper is deleted. - `SectionContext::teardown` consumes the frame and stays a method rather than `Drop`, because a fanout arm's VM must outlive its cancel-scoped body. - The live H1 pass and the fanout arm keep their own preambles and locals in this step.
`execute_live_h1` becomes a construct-run-teardown cycle over a `SectionContext` built by the new `new_live_h1` constructor, which absorbs the `sys` JSON (id 0 under the prompt's title), VM construction and limits, host injection, the host APIs, the control-global stubs, and the infer hook carrying the live binding producer. State extraction reads `var` out of the frame through `read_var`. - The H1 frame seeds an empty `var`, no reply, no item, no write scope, and `execute_depth` 0; H1 installs the stubs, so no depth check ever reads it. - The fanout arm still keeps its own preamble and calls `run_one_section_impl` from locals.
`run_one_arm` becomes a construct-run-teardown cycle over a `SectionContext` built by `new_fanout_arm`, seeded with the arm's item, write scope, indexed `sys`, cloned `var`, and one-deeper execute depth. The fanout's proxy observer, proxy debug sink, and fresh turn counter arrive as frame fields instead of through a forged context. - `ArmInputs` grows by `observer`, `debug`, and `turns`, because those handles must cross the spawn boundary to seed each arm's frame. - Frame construction runs inside the arm's cancel scope via `cancel::maybe_scope`, because the setup preamble executes Lua and the instruction hook reads the task-local cancel handle. - `RunFrame.item` is deleted; the arm was its only reader. - The `FAIL_ARM_VM_SENTINEL` test hook now fails frame construction up front instead of failing `SectionVm` construction.
`RunContext` now carries all run-scoped state, with derived `max_tool_iterations` and `section_count` methods, and `make_control_globals` and `drive_contained_chain` capture `RunContext` clones directly. The `run_sections` frame-rebuild fork is gone, and `run_fanout_arms` takes `&RunContext` plus the fanout call's own inputs as parameters. - Three sanctioned forks replace the deleted contexts: `with_walk_state` at the H1-to-walk handoff, `with_effective_handles` for a fanout's proxy reporting handles, and `with_args` scoping an `execute` call's input over its chain. - A new test pins that a nested `execute` without input inherits the chain's args and the run's args never leak in. - `RunContext` trades its derived `Debug` for a hand-written one that hides the trait objects. - `ArmInputs` shrinks to the spawn-boundary bundle.
The prompt-language verbs `tools.need` and `models.need` rename to `tools.bind` and `models.bind`, so each verb matches the `ToolBinding` the call records. The change breaks the prompt language on purpose and changes no behavior; prompt files, fixtures, guides, and error messages update throughout. - Supporting symbols rename with the verbs: `ModelNeedOpts` becomes `ModelBindOpts`, `parse_need_args` becomes `parse_bind_args`, and `record_need_binding` becomes `record_bind_binding`. - Error text that named the old verbs now names `tools.bind` and `models.bind`, including the H2 stub errors.
`ToolBinding` now holds `tool: Arc<dyn Tool>`, attached at bind time during live H1 resolution, so schema preparation and dispatch read through the binding and never consult the implementation catalog after H1. A capability whose tool is unavailable fails at the `tools.bind` call site, so `Error::UnknownScopedTool` is unrepresentable and deleted. - The dispatch map becomes alias to `DispatchTarget` (`Bound` or `Local`) instead of alias to identity, retiring the `local` sentinel; `ToolId::is_local` is deleted. - `ToolBinding` gets hand-written `PartialEq`, `Eq`, and `Debug` keyed on its data fields; the attached trait object takes no part in comparison. - `SharedTools` replaces its `registry` view with a `get` lookup returning `Arc<dyn Tool>`, a linear scan on a cold path; `ToolRegistry::from_unique` is deleted. - `run` and `execute_live_h1` no longer thread a `ToolRegistry`; `RuntimeResolution::new` takes the shared tools instead.
`SharedTools` becomes `ToolCatalog`, a public, validated, caller-constructed catalog mirroring `ModelCatalog`, and folds into `ResolutionContext` beside the picker and the model catalog. `run` no longer takes a tools slice; duplicate-id and wire-name validation fail at the caller's construction site. - `ToolRegistry` folds into `ToolCatalog` and is deleted; `ToolRegistryError` and `ToolRegistryErrorKind` rename to `ToolCatalogError` and `ToolCatalogErrorKind`. - `Error::DuplicateLiveToolId` and `Error::InvalidToolWireName` are deleted; a catalog construction failure no longer converts into the run's error enum. - `RunContext` drops its `shared_tools` field and accessor; `ResolutionContext` gains `tools`, and `execute_live_h1` reads the catalog there.
`ToolBindings` becomes `ToolSet` behind `Arc<Mutex<ToolSet>>`, with the read-only `ToolView` trait implemented directly on the mutex, and `RunContext` holds `Arc<dyn ToolView>` created empty at run start, so the tool set rides across the H1-to-H2 boundary with no fork. Near-duplicate conflicts are computed per `tools.bind` and recorded symmetrically on both bindings as `conflicts: Vec<Conflict>`. - Scope validation becomes a local check: a clash errors when both halves of a recorded conflict enter one model-visible scope, and `ToolAnalysis` is deleted. - `ToolResolver` gains a `near_duplicates` method whose default reports no pairs, so a resolver without similarity knowledge records no conflicts. - `Conflict` compares the similarity score by bits, keeping equality reflexive at NaN for `ToolBinding`'s `Eq`. - The never-emitted `ToolRegistryValidationStarted`, `ToolRegistryValidationSucceeded`, and `ToolRegistryValidationFailed` observation variants are removed. - A new test pins that two clashing binds each record the other alias with the picker's score.
`ModelBindings` becomes `ModelSet` behind `Arc<Mutex<ModelSet>>` with the read-only `ModelView` trait implemented directly on the mutex, mirroring the tool side. `RunContext` creates the set empty at run start and the live H1 pass writes through the concrete `model_set` handle, so `with_walk_state` loses its models delta and only stamps `when`. - `ModelSet` exposes `default` as a field with no inherent accessor, because a `default()` method would shadow `Default::default()` at construction sites. - `attach_infer_hook` now passes the shared `Arc<dyn ModelView>` into the infer hook in place of `None`. - `ModelSet` keeps `PartialEq` without `Eq`, because bindings carry `f64` temperatures. - No conflict analysis comes to the model side; `ModelView` carries only the read methods.
The `convert` function in `build.rs` reads `source` four bytes at a time. The loop now iterates `as_chunks::<4>().0` and passes each chunk to `u32::from_le_bytes` by value. - The loop still covers only complete four-byte chunks; trailing bytes stay unused.
Move configuration loading out of the gateway into a new `promptforge-gateway-config` crate so tooling can parse and validate a gateway configuration without the HTTP stack. The move carries the `config` and `profile` modules, the config error types, `QueueConfig`, and `default_promptforge_root`; the gateway depends on the new crate and re-exports `Config`, `Secret`, and `ProfileName`. - The crate adds a new public error surface: an opaque `ConfigError` over a private representation, classified by `ConfigErrorKind`, so no `toml` or `io` type appears in a public signature. - `list_profiles` becomes public in the new crate. - `Gateway`, `ServeOptions`, and `run` are unchanged; the gateway crate drops its `dotenvy` and `toml` dependencies.
Make the configuration crate's types unconstructable and unreadable except through validated construction and getters, so the representation can change without breaking callers. Every `pub` field on the config structs becomes private, a new `config/accessors.rs` module provides one documented getter per field, and `#[non_exhaustive]` lands on each public struct and enum, including `ProfileName`. - `Secret` stays opaque: accessors such as `api_key()` return `&Secret`, and the value is reachable only through `expose()`. - `QueueConfig` gains `max_depth()` and `fair_scheduling()` accessors; validation now reads through them. - The gateway call sites in `local/mod.rs`, `routing.rs`, `tools.rs`, and `queue.rs` switch from field reads to the new getters.
`PromptForgeServer` builds its tool list synchronously in `list_page`. `list_tools` now returns `impl Future<Output = Result<ListToolsResult, ErrorData>>` through `std::future::ready` instead of an `async fn` body. - `call_tool` stays an `async fn`; only `list_tools` changes shape. - No tests change.
The golden tests compare `serde_json::to_string_pretty` output against JSON files included with `include_str!`. Each test now runs `replace` on the golden text to convert `\r\n` to `\n` before `trim_end` and `assert_eq!`. - Both golden tests, picker and no-picker, get the same normalization. - The golden JSON files themselves do not change.
Make boot always name a profile: `--profile NAME` is required and parsed into a `ProfileName` at parse time, the config path is one optional positional with `PROMPTFORGE_GATEWAY_CONFIG` as the fallback, and `--profiles-dir` is removed. Path resolution goes through a new pure `resolve_config_path` helper so tests pass both sources explicitly and never touch the process environment. - As an interim mapping, `parse_args` still builds `ServeOptions` from the config file's sibling `profiles/` directory and `ConfigSource::Profile`. - The old mutual-exclusion rule between `--profile` and a config path is deleted; both are now required inputs to boot. - New tests pin CLI-wins, env fallback, neither-set, missing `--profile`, invalid and traversal profile names, and rejection of the removed `--profiles-dir` flag.
Boot now loads the named profile from the boot file's sibling `profiles/` directory and fixes the socket and bearer key for the process lifetime. `ServeOptions` now holds a `config_path` and a `profile`, and `ConfigSource` is deleted; boot loads the profile's env file then the boot file's, resolves the profile through its include chain, and rejects the boot when the profile's merged `[server]` differs from the boot file's by value. - `check_server_matches_boot` compares `bind` and `api_key` as interpolated values: a `bind` mismatch names both addresses, an `api_key` mismatch redacts both keys. - The config crate no longer loads env files: `load_env_chain` and `default_profiles_dir` are deleted, the `dotenvy` dependency moves to the gateway binary, and a new `load_server` reads only a boot file's `[server]` section without full validation. - A profile switch loads the new profile's env file and re-checks `[server]` against the retained boot server before building routing; a mismatch fails with `switch_failed` and leaves the live profile intact, pinned by a new integration test. - Boot logs the resolved include chain and warns when the boot file is not in it.
Bring the user-facing documentation in line with the boot rules: the `serve` invocation with a mandatory `--profile`, the `PROMPTFORGE_GATEWAY_CONFIG` fallback, the sibling `profiles/` directory, and the boot-owned `[server]` rule. The change touches `README.md`, the gateway crate readme, the two user-guide copies, and `guide/src/gateway.md`. - The docs add the two-env-file rule: the profile's env file loads first, then the boot file's, neither overrides the process environment, and included files' env files are never loaded. - The docs describe the catalog-vs-selection split: the boot file is the catalog, and a minimal `profiles/main.toml` containing only `include = ["../gateway.toml"]` loads the full catalog. - Documentation only; no code changes.
Remove documentation and comment references to machinery the boot rework deleted. `design/design-gateway.md`, `promptforge.md`, `profiles/base.toml`, and a doc comment in `local/error.rs` now describe the sibling `profiles/` directory, the mandatory `--profile`, and the two-env-file rule instead of `--profiles-dir`, `default_profiles_dir`, and include-chain env loading. - The `local/error.rs` doc comment is the only Rust file touched; there are no code changes.
Introduce `[[dominion]]` as the new admission-control configuration: named pools of compute with a shared concurrency limit, a bounded queue, and a VRAM budget. The change adds `DominionConfig`, `DominionKind`, and `QueuePolicy` plus additive `dominion`, `parallel`, and `vram_gb` fields on endpoints and local models, each with read accessors and doc examples. - Validation rejects a duplicate or empty dominion id, `max_concurrency` or `max_queue` below 1, `vram_gb` on a remote dominion, and a binding to an undefined or wrong-kind dominion. - `parallel` on `LocalModelConfig` stays `Option<u32>` so an unset field cannot override a legacy lane's concurrency with the default. - The new fields are parsed and validated but nothing reads them at runtime yet; the legacy `concurrency`, `device`, `lane`, and `[queue]` keys still parse and govern runtime limits.
Rename the admission controller from `EndpointLane` to `DominionQueue` and give it the queue-vs-reject policy: under `QueuePolicy::Reject` a full in-flight set fails the admit immediately instead of enqueueing. The new `AdmitError::Rejected` maps to `GatewayError::QueueRejected`, answered as 429 `rate_limit_error` so an OpenAI client sees a retryable rate-limit error rather than a server failure. - `DominionQueue::new` now takes `max_depth`, `fair_scheduling`, and `policy` as separate parameters; a `from_queue_config` shim keeps the legacy `[queue]` settings working, always with the `Queue` policy. - Fairness stays per-client round-robin keyed by the self-asserted `X-PromptForge-Client` header, with no discipline abstraction. - Call sites in `lib.rs`, `local/mod.rs`, and `routing.rs` are mechanical renames; new tests pin fail-fast rejection at capacity and the 429 mapping.
The both-present arm of `check_workshop_matches_boot` reported only that "the profile's workshop settings differ", while the adjacent `check_server_matches_boot` names the exact differing field and its values. The check now compares the four fields in declaration order through a new `first_workshop_difference` helper and names the first difference in the validation message. The message keeps the "[workshop] mismatch" prefix the existing tests assert on. - `bind` and `open_browser` print both values; `voice` and `tape` print both Debug forms, and neither carries a secret. - The helper's fallback arm is unreachable until the config grows a field the check does not name yet; its doc says so. - A new test `workshop_mismatch_names_the_first_differing_field` exercises all four fields and asserts both values appear for `bind`.
The no-workshop stub of `spawn_if_configured` suppressed `clippy::unnecessary_wraps` with `#[allow]`, which stays silent forever. The suppression is now `#[expect]` with the same reason, so it warns once it goes stale, for example if the stub ever gains a fallible path. - Verified against `-D warnings` in both feature configurations: the lint still fires in the no-workshop build, so the expectation is fulfilled, and the workshop build compiles the hosted variant instead.
The `open_browser` honor called `open::that` on the workshop URL inline, so no test could observe it without opening a real browser. `spawn_if_configured` splits into a thin production wrapper that passes `open::that` over a new `spawn_with_opener` core that takes the opener as a plain closure. - The opener parameter is `impl FnOnce(&str) -> std::io::Result<()>`, a closure injected per call rather than stored state. - Three tests cover the honor: `the_open_browser_honor_opens_the_workshop_url`, `the_opener_never_runs_without_the_open_browser_honor`, and `a_failing_opener_does_not_fail_the_spawn`. - Each test spawns a real workshop on an ephemeral loopback port with the tape anchored in a tempdir, matching the runner's existing spawn fixtures.
`GatewayHandle::shutdown` stops a hosted workshop, waiting out its bounded drain, before sending the gateway's graceful-shutdown signal, so the workshop's final gateway calls never hit a dead socket. No test asserted that order. A test-only `mpsc` observer on `GatewayHandle` now records `ShutdownStep::WorkshopStopped` after the drain returns and `ShutdownStep::GatewaySignaled` after the shutdown send. - The seam is `cfg(test)`-gated: the `observer` field, the `ShutdownStep` enum, the `observe_shutdown` setter, and the two `record` calls vanish from production builds. - Both records are synchronous inside `shutdown()`, so the tests collect with `try_iter` and carry no timing dependence. - `shutdown_drains_the_workshop_before_signaling_the_gateway` asserts the two-step order; `shutdown_without_a_workshop_signals_the_gateway_only` covers the no-workshop path, which also keeps the seam used in every test build.
The gateway's startup path called `load_server` and `load_workshop` on the boot file back to back, and each ran its own `collect_config_chain` plus `${VAR}` interpolation, so the same include tree was read, parsed, and merged twice per boot. `promptforge-gateway-config` gains `load_boot_sections`, which resolves the chain and interpolates once, then extracts the `[server]` and optional `[workshop]` sections from the same document. `load_startup` in the gateway now makes one boot-file pass instead of two, with the parity checks unchanged.
- Section extraction moves into shared `server_section` and `workshop_section` helpers used by all three loaders, so the single-section entry points keep their exact behavior: `load_server` still requires `[server]`, and `load_workshop` still returns `None` for a missing section even when `[server]` is absent.
- The combined loader deliberately does not back `load_workshop`: a workshop-only file has no `[server]`, and the existing include-chain test for that case pins the tolerant behavior.
- New tests cover the combined loader: `load_boot_sections_reads_both_sections_without_full_validation`, `load_boot_sections_returns_none_workshop_when_the_section_is_absent`, and `load_boot_sections_requires_a_server_section`.
The shell's `workshop_url` mapped the gateway handle's `Option<&str>` to the window URL inline, so the user-facing error arm - a boot config with no `[workshop]` section, leaving the shell with no page to open - had no test coverage. The mapping now lives in `workshop_url_from`, a pure `Option<&str>` to `anyhow::Result<String>` function that `workshop_url` delegates to. Behavior is unchanged: the same context message, the same `Ok` passthrough. - `workshop_url_from_passes_the_url_through` pins that a present URL passes through untouched. - `workshop_url_from_names_the_missing_workshop_section` pins that the `None` arm's error names the `[workshop]` section, so the message cannot drift into telling the user nothing actionable.
The shell's README sends readers to the gateway README for the field reference, and the `[workshop]` tables landed, but the required `[server]` section's `bind` and `api_key` were documented nowhere in it. A short "The `[server]` section" table now sits between Usage and Hosting the workshop: both fields required, both accepting `${VAR}` interpolation, with a pointer to the boot-ownership rule the two sections share.
`serve_thread` wrapped a tokio runtime build failure in `StartupError::bind`, whose Display reads "failed to bind the listener", misnaming what actually failed. The runtime build now reports through `StartupError::thread`, and the `Thread` kind's doc widens to name the runtime build alongside the spawn, exit, and panic cases. - Forcing a real runtime build failure takes resource exhaustion, so the path is covered by the existing kind-mapping test on the constructor rather than a dedicated spawn fixture. - The kind enum is `#[non_exhaustive]`, so the widened doc is the only API-surface change.
`failed_handshake`'s payload reader had three arms and a test for one: the panicked-thread fixture exercises the `&str` payload, but the owned `String` arm (a `panic!` with format args) and the non-string fallback (a `panic_any` call) were new behavior nothing would catch breaking. One unit test now calls `panic_message` directly with each payload shape. - `panic_message_reads_each_payload_shape` passes a borrowed `&str`, an owned `String`, and a `u64` standing in for a `panic_any` payload, pinning the "non-string panic payload" fallback text.
The `send` feature's borrow tracking relied on compiler internals and broke on newer rustc; the fix shipped in mlua 0.12 and was never backported to 0.10, so the pin had to move before the coroutine work touches `lua/`. `Cargo.toml` bumps `mlua` to 0.12 with features `lua55`, `vendored`, `serialize`, and `send`; the `async` feature stays off. The breakage is mechanical: `set_hook` and `set_metatable` are fallible in 0.12, so their call sites now propagate `Result`. - `install_instruction_budget` now returns `Result<()>`; VM construction propagates a hook-install failure through the existing `construction_failed` path. - The `set_metatable` calls in `crates/promptforge-core/src/lua/sys.rs` and `crates/promptforge-core/src/lua/tools_bridge.rs` now propagate with `map_err(Error::lua)`. - No test logic changed: one call site in `crates/promptforge-core/src/lua/tests.rs` gained an `expect` for the fallible hook install. Stale doc comments saying Lua 5.4 now say 5.5.
Every exit path out of a section entry must tear the VM down exactly once, and the driver's explicit `SectionContext::teardown` calls left that to each caller. `SectionContext` now owns the boundary through a `Drop` impl with an armed/disarmed `completed` flag: the success path calls `mark_completed` after the final `var` read-back, so the drop fires the `LUA_TEARDOWN_STARTED`/`LUA_TEARDOWN_SUCCEEDED` pair on every path and `SECTION_FINISHED` only when armed. The frame holds its VM as `Option<SectionVm>` plus its `name` and `execution`, so the destructor needs no parameters. - The constructors run `setup_section_vm` on the bare VM before the frame exists; a setup failure calls `vm.teardown` directly and no `SectionContext` is ever created. - The `h1_try!` macro in `crates/promptforge-core/src/execute/h1.rs` is gone; the fanout arm replaces `frame.teardown(&worker.name)` with `drop(frame)` at the same point, so the teardown pair still precedes the arm's terminal observation. - The write-only fields `write_scope` and `execute_depth` are removed; their values reach the control globals and the VM setup through locals. - The live H1 frame is never marked completed, so `SECTION_FINISHED` stays a walked section's boundary and never fires for the setup pass. - A new test, `an_erroring_section_tears_down_exactly_once_without_finishing`, pins the error path: the teardown pair fires exactly once and `SECTION_FINISHED` does not fire.
The yield/resume boundary needs validated message types: what a script can cause the host to do must be one short read with compiler-checked per-variant fields. New module `crates/promptforge-core/src/execute/protocol.rs` defines a closed `Request` enum with all four variants (`Infer`, `Execute`, `Fanout`, `Mcp`) and an `Answer` enum that renders the `(ok, result)` resume envelope. Validation is strict: `Request::from_yield` checks every field at the trust boundary, and any yield that is not a well-formed request table fails with the fixed `Error::Lua` message "scripts may not yield directly". - Field reads go through `raw_field`, so a script-space metatable cannot intercept or forge a request field. - Each `Answer` variant owns its typed `Error` until `into_envelope` consumes it, returning the rendered envelope plus the typed error, so the driver never sees a stringified failure; this holds for leaf and structural variants alike. - A received `Mcp` request is a typed protocol error through `mcp_reserved()`: the variant is reserved, not dispatched. - Nothing outside the module calls it yet; `pack_sequence` in `crates/promptforge-core/src/lua/vm.rs` was promoted to `pub(crate)` for the fanout envelope rather than duplicated. - Twenty-one unit tests cover well-formed parses, malformed rejections, and envelope round-trips.
Yield cannot cross the C boundary, so the host calls that suspend must become Lua shims that yield request tables. The new `crates/promptforge-core/src/lua/__impl_coro.lua` turns `models.infer`, `handle:infer`, and `execute` into `coroutine.yield` wrappers that consume the `(ok, result)` envelope and raise failures with `error(result, 0)`, so no position prefix leaks into script errors. `setup_section_vm` gains a `VmSetupMode`: `Legacy` (the default) keeps the existing Rust control globals, and `Scheduler` installs the shims after the host tables exist and before `replay_shared`. - The shim source is embedded with `include_str!`, compiled once behind a `LazyLock` through `LuaProgram::compile_internal`, and named `@crates/promptforge-core/src/lua/__impl_coro.lua`, so shim errors render as verbatim file:line references the line mapper never rewrites. - Model handles reach author code as sealed proxy tables through `wrap_handle`: `infer` is a Lua method that yields the inner userdata, and `__metatable` seals the proxy so `getmetatable` cannot hand the unshimmed userdata back to author code. - The `coroutine` standard library loads at shim-install time and the global is stripped again before the install returns, so author code cannot yield directly; legacy VMs never load it. - `SectionVm` gains a `coro_shims` flag so the captured model alias globals install as shim-wrapped proxies; `var_snapshot_table` in `crates/promptforge-core/src/lua/sys.rs` materializes the guarded `var` as a plain table for the `execute` request's snapshot. - No production caller constructs `Scheduler` mode yet, and there is no `fanout` shim; the change scopes itself to `infer` and `execute`. Seven focused tests cover well-formed yields, prefix-free error raising, unmapped shim frames, and proxy sealing.
The scheduler needs a chunk execution path that can suspend on a shim yield, so `SectionVm` gains a resume-based path next to the legacy `Function::call` path, which stays untouched. `start_block_coro` creates one coroutine per Lua block on the section's persistent VM, `resume_block_coro` drives a suspended block, and the new `CoroStep` enum (`Yielded` or `Done`) reports the boundary. Jump-slot precedence, runtime-error mapping, and scalar-return handling match the legacy path, so block results and rolled-forward VM state are indistinguishable. - Instruction hooks are per-coroutine in PUC Lua, so the budget/cancellation hook moves into `InstructionBudget`, a shared `Arc<AtomicU64>` counter installed on each block coroutine through `Thread::set_hook`; one counter spans every chunk of a section and now bites inside resumed coroutines. - Six focused tests pin the resolved spikes: the hook fires inside a resumed coroutine, the budget spans block coroutines on one VM, a yield crosses `pcall`, `jump` propagates through `Thread::resume` unchanged, `@`-prefixed chunk names render verbatim through resume, and scalar returns plus `var`/`reply` state roll forward across blocks. - No production caller exists yet: `start_block_coro`, `resume_block_coro`, and `CoroStep` carry dead-code allowances until the scheduler's driver loop consumes them.
Section execution needs a driver that resumes a chain's coroutine, matches the yielded request, dispatches it, and resumes with the answer, all on one thread. New module `crates/promptforge-core/src/execute/scheduler.rs` adds `Scheduler`, which owns a chain arena indexed by a `ChainId` newtype, a LIFO execute stack, a FIFO ready queue, a pending table mapping in-flight requests to chains, and an unbounded answer channel fed by `spawn_local` infer tasks. A `Chain` owns its `SectionContext`, position, in-flight block coroutine, walk-scoped `reply`/`var` slots, and a per-chain `execute_depth` field. - `Scheduler` lives entirely in the driver loop's stack frame - no `Arc`, no `Mutex`, unreachable from Lua; `RunContext` stays the ambient read-mostly context and the two are deliberately not merged. - The recursion cap reads the chain's `execute_depth` field, never the stack length, because fanout arms will increment depth without sitting on the execute stack. - Dispatch failures (the depth cap, target resolution, child construction) resume the caller through the error envelope, so an author `pcall` catches them exactly as on the legacy callback path. - `Infer` dispatch spawns one gateway round through the new `infer_round`, which shares its reporting tail with the legacy hook through the extracted `accept_infer_completion`; `Execute` pushes and pops the chain stack; `Fanout` and `Mcp` fail with the typed reserved errors `fanout_reserved()` and `mcp_reserved()`. - The driver body runs inside a `tokio::task::LocalSet` and selects on the answer channel and the cancellation notification; cancellation aborts the in-flight I/O tasks and returns `Error::Interrupted`. - `run_section_prose` was extracted from `run_one_section_impl`, so the scheduler's driver and the legacy block loop run the identical prose path. - A completed jump still errors the chain (the walk translation lands later), the `JoinState` join table is defined but unused, and no production caller exists until the flip. Six `current_thread` tests cover the nested-execute-plus-inference gate, cancellation while suspended on infer, the depth cap, prose client seeding, reply read-back, and dispatch failure into `pcall`.
The scheduler now runs the walk's core rules as chain transitions, so a run advances through sections in document order with walk-scoped state. `Chain` gains the `reply`, `var`, and `addressed` slots, and frame construction moves out of `start_chain` into a new `enter_section` transition. At a section's end, `end_section` reads the final `reply` and `var` back while the VM is live and rolls them forward. - `enter_section` skips off-walk sections on fall-through before any frame exists; an addressed arrival (an execute target) runs its section anyway, and one entry consumes the flag. - `Chain.frame` is now `None` before the first entry and between sections; `finish` takes the chain's `reply` slot as the result text when the walk runs off the slice's last section. - `end_section` arms the frame with `mark_completed` before the drop, so the teardown boundary fires `SECTION_FINISHED` for the completed section. - An execute chain's `var` slot seeds from the caller's snapshot and is discarded with the chain, so the caller never sees the chain's writes. - A jump still returns the typed later-step `Error::Lua`; a received `fanout` or `mcp` request stays the protocol's reserved error. - The change touches only `execute/scheduler.rs` and its test module; fifteen new scheduler tests pin fall-through order, off-walk skips, reply and `var` roll-forward, and the run-global id counter.
The scheduler's walk now applies a Lua `jump` as a control transfer instead of failing the chunk with the placeholder `Error::Lua`. `Chain` gains a `positions` stack of suspended parent walk positions: `apply_jump` reads `reply` and `var` back from the jumper's live frame, marks the frame completed, resolves the heading with `resolve_jump_target`, and moves the walk. A `JumpTarget::Sibling` sets the chain's index within its slice, addressed; a `JumpTarget::Child` pushes the current position and descends into the jumper's child slice, and `pop_position` resumes the parent after the jumper when the child level exhausts. - The jumper's frame closes as completed before the target resolves, so `SECTION_FINISHED` fires for the jumper and an author's `reply = nil` steers what the target sees; a failed resolution returns `Error::Lua` after that close. - A scalar return inside a descended child level ends the whole chain, and the run-global id counter counts entries across descents. - Twenty-six new tests in `execute/tests/scheduler.rs` mirror the legacy jump, return, and observation cases; the legacy suite is untouched, and a received `fanout` or `mcp` request still fails with the typed reserved error.
The scheduler can now run the prompt's live H1 pass as the driver loop's first chain, before the root walk. A `Chain` marked with the new `h1` field holds the prompt's H1 blocks: `start_live_h1` builds the frame through `SectionContext::new_live_h1` with `VmSetupMode::Scheduler`, each Lua block steps through `h1_scoped_step` inside a fresh resolver scope, and each prose block runs through `run_live_h1_prose`, extracted from the legacy block loop so both drivers share one path. `end_live_h1` reads the final `var` and reply back, drops the frame unarmed, and starts the root walk from the `var` hand-off - or makes the pass's reply the run result when the prompt has no sections. - `SectionContext::new_live_h1` gains a `mode` parameter: `VmSetupMode::Legacy` keeps the bridged infer hook, while `VmSetupMode::Scheduler` installs `install_live_h1_shim_base` and re-wraps each block's live models table through `shim_live_h1_models` and the new `__impl_coro_h1.lua`. - `finish_h1_step` applies the chunk outcome first and reports a captured resolver callback error after it, so a `pcall`-caught callback error still observes `LUA_CHUNK_SUCCEEDED` before the run fails. - The pass keeps id 0, fires no section observations, and never arms completion, so `SECTION_FINISHED` cannot fire for it; a scalar return short-circuits the whole run. - No production caller arms the pass yet: `run` is unchanged, and the legacy `h1.rs` path only passes `VmSetupMode::Legacy`. Eighteen new tests in `execute/tests/scheduler.rs` cover the pass's rules; the legacy suite is untouched.
The scheduler now dispatches the protocol's `Fanout` request. `prepare_fanout` checks the depth cap, rejects an empty collection, and resolves the worker through `resolve_chain_target`; `refill_fanout` then starts one arm chain per collection member while a slot of the run's `max_fanout_concurrency` window is free. Each arm is an ordinary chain over the worker's slice, seeded from the join's shared `ArmTemplate`, and `complete_arm` writes the arm's preallocated per-index slot, so the blocked parent resumes with the results in collection order when the last arm lands. - `JoinState` gains the window accounting (`active`, `next`, `items`, `window`) and the `ArmTemplate`; `Chain` gains the `arm` field, and an arm's first entry builds its frame through `SectionContext::new_fanout_arm` with `VmSetupMode::Scheduler`. - An arm still at its worker resolves control transfers through `resolve_arm_target` over the caller's visible set minus the worker plus the worker's children; a sibling-level target walks its own prompt slice, not a materialized home `Vec`. - The `fanout` yield shim installs in scheduler mode from `__impl_coro.lua`, and `Request::fanout_reserved` is deleted; `legacy_mode_keeps_the_rust_fanout_global` pins that Legacy mode keeps the Rust callback. - A fatal arm error only fails the join and discards late siblings: `fail_fanout` aborts nothing, and its doc comment defers the sibling abort to later work. The legacy fanout driver and its suite are untouched, and a received `mcp` request still fails with the typed reserved error.
The scheduler's fanout now applies the legacy engine's failure contract. Each arm chain carries an `ArmFinalizer`, so exactly one terminal observation fires per arm: `complete_arm` finishes it with the arm's real outcome, and its drop reports `FANOUT_ARM_CANCELLED`. A fatal arm error fails the join and aborts the sibling subtree through the new `abort_subtree`, which recurses through execute children and nested fanouts, removes the chains from the ready queue and the pending table, and aborts their in-flight I/O tasks. - `io_tasks` becomes a `HashMap` keyed by `RequestId`, so a fatal arm can abort a sibling's own in-flight round and each handle drops with its entry; an answer that arrives for a request with no pending entry is discarded. - `Error::ToolLoopExhausted` soft-degrades its arm to the incomplete stub through `LuaFanoutResult::exhausted_stub` and finishes with `FANOUT_ARM_EXHAUSTED`, so one stuck arm cannot kill the sibling results. - Arm creation observes `FANOUT_ARM_STARTED` at the dispatch boundary, and `fanout/mod.rs` newly exports `ArmFinalizer`. - Twelve new tests in `execute/tests/scheduler.rs` pin the depth cap, the empty-collection and list-section rejections, the store write-write race, legal appends, sequential fanouts sharing a path, the sibling abort, the soft degrade, and cancellation while suspended in an arm; the legacy fanout suite is untouched.
`run` now drives the whole prompt - the live H1 pass, the walk, execute chains, and fanout - through `Scheduler::new`, `with_live_h1`, and `drive`, and the legacy engine is deleted in the same commit. Removed: the `engine.rs` walkers (`run_sections`, `walk_siblings`, `run_one_section`), `drive_contained_chain`, `run_one_section_impl` with `BlockRunMode` and `SectionFlow`, `execute_live_h1` and the `h1` module, the `JoinSet` fanout driver and the fanout proxies, the `VmSetupMode` split, `attach_infer_hook`, and `bridge_blocking` with its current-thread guard. With no spawned arm tasks left, infer I/O dispatches through `tokio::spawn` instead of `spawn_local`, and the driver checks `cancel::is_cancelled()` at every chain-step boundary. - `Request::from_yield` now returns `YieldParse`: an author argument error becomes `YieldParse::Call` and rides back as the call's error answer, so the shim raises it at the call site and a `pcall` catches it; only a malformed yield table fails the block with the direct-yield message. - `Error::FanoutArmJoin` is deleted: with no spawned arm tasks there is no join failure to represent, and its `RunErrorKind` and `CompletionErrorKind` mappings go with it. - The driver loop no longer runs inside a `LocalSet`, and the frame constructors lose their mode, client, and depth parameters because scheduler mode is the only setup path. - The removed `fanout/tests.rs` cases pinned the deleted driver; the arm-item shared-replay case reappears on the scheduler as `the_shared_replay_sees_the_arm_item`, joined by the new pins `pre_cancelled_fanout_returns_interrupted` and `model_required_when_arm_prose_has_no_binding`.
A run no longer needs any particular Tokio runtime flavor: the `execute` module docs gain a `# Runtime` section, and the doc example on `run` now exercises `execute` on a current-thread runtime. `promptforge-cli` and `promptforge-dev` move to `#[tokio::main(flavor = "current_thread")]`, and the `cancel` module docs lose their stale `block_in_place` references. Two driver fixes ride along: `prepare_fanout` now tears the join down when a mid-refill arm start fails, and an answer for an unknown request id fails loudly again because `abort_subtree` records the ids it aborts in the new `aborted_requests` set. - `Scheduler` gains `max_chains` (default `u32::MAX as usize`) so the `start_chain` overflow path is testable through the new `set_max_chains_for_test`; `post_answer_for_test` posts a phantom answer to drive the unknown-id path. - The `fanout-store-writes.md` fixture's rendezvous now yields through `execute` on a nop `## Yield` section instead of busy-polling the store, so the anti-sequential pin holds when the arms share one thread. - Two new tests in `execute/tests/scheduler.rs` pin both fixes: `a_mid_refill_arm_start_failure_tears_down_the_join` and `an_answer_for_an_unknown_request_id_fails_loudly`. No other crate changes runtime flavor.
Member
Author
|
rewrote the entire history of commit messages, and merged it. |
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
3 participants
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
This batch grows the Workshop from a chat panel into a real desktop application. It adds a decorationless Windows shell with a custom title bar and menus, confined workspace file access with a file-tree panel, a CodeMirror 6 editor surface with layout persistence, and a round of chat UX polish covering streaming performance, thinking blocks, and tool activity.
Desktop shell
the tao shell
and Help menus, skinned to match the rest of the UI
Workspace files
the workspace root
Editor
EditorSurfaceinterface, so theediting surface can be swapped without touching the dock layout
shortcuts
Chat UX
stop reparsing finished content
loader is gone in favor of immediate prefill
timestamp tooltip
run on a GPU
Docs
Test plan
cargo test --workspaceand strict clippy stay green