feat: add shared ExecutionBudget consolidating runtime resource caps - #609
Conversation
Replace a dozen isolated, per-subsystem resource ceilings (and several that were simply unbounded) with one coherent ExecutionBudget object that travels with a run through parsing, evaluation, pattern matching, web handling, and module loading. The budget owns every dimension: - Deadline and cooperative cancellation (was Interpreter.max_duration/op_count) - Interpreter operation ceiling (new, opt-in; default unlimited) - Recursion and import depth (recursion was unguarded in release) - execute-file depth (was MAX_EXECUTE_FILE_DEPTH) - Pattern transitions and active states (was MAX_STEPS; states now bounded) - Source, request-body, and response byte caps (source/response were unbounded) - Pending HTTP requests (was web_server_request_queue_bound) - WebSocket queue and connection limits (were unbounded channels/registry) Design: - src/exec/budget.rs: ExecutionBudget is Send+Sync (atomics only), so an Arc clones into the multi-threaded web transport without any Rc/RefCell crossing a thread boundary. The interpreter core stays !Send. BudgetLimits maps the existing .wflcfg keys plus nine new budget keys; BudgetExceeded keeps the historic timeout wording/ErrorKind so existing handling matches. - The interpreter runs on a dedicated 1 GiB-stack thread so max_call_depth (default 1000) turns runaway recursion into a clean, catchable error instead of a native stack overflow (an 8 MiB stack overflows near depth 40). Backward compatibility: the three pre-existing knobs keep their defaults exactly; new ceilings default generously; max_operations is off by default. All 515 lib tests, the new budget suite, and 107 TestPrograms pass. Docs: configuration-reference (new keys + Execution budget section), web-servers (response/WebSocket limits), and a Dev Diary entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change introduces a shared ChangesShared execution budget
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ExecutionBudget
participant Interpreter
participant PatternVM
participant WebTransport
CLI->>ExecutionBudget: construct limits from configuration
Interpreter->>ExecutionBudget: charge operations and acquire request capacity
Interpreter->>PatternVM: execute pattern with shared budget
PatternVM->>ExecutionBudget: charge steps and reserve active states
WebTransport->>ExecutionBudget: reserve bytes and connections
ExecutionBudget-->>Interpreter: return permit or typed budget error
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Pattern find/find_all silently swallow cancellation and budget errors
The PatternVM::find method at src/pattern/vm.rs:201-205 uses if let Ok(Some(result)), meaning any Err (including PatternError::Cancelled or StepLimitExceeded) is silently treated as 'no match at this position' and the loop tries the next position. The same applies to find_all at line 224. This is pre-existing behavior (the old code also swallowed StepLimitExceeded), but it's now more significant because the budget can inject cancellation errors. A cancelled pattern match will appear to return 'no match' rather than propagating the cancellation. The _with_budget variants on CompiledPattern inherit this behavior. The interpreter's matches operator (src/interpreter/mod.rs) calls matches_with_budget which also returns false on error via unwrap_or(false). This means a budget cancellation during pattern matching silently produces a false-negative rather than an error.
(Refers to lines 192-209)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared, cross-cutting exec::budget::ExecutionBudget to centralize and consistently enforce runtime resource ceilings across the interpreter, pattern VM, module loading, web handling, and CLI entry points, replacing multiple scattered limits and several previously-unbounded resources.
Changes:
- Adds
src/exec/budget.rswithBudgetLimits,ExecutionBudget, typedBudgetExceeded, and RAII guards for request/WS connection accounting. - Threads a shared
Arc<ExecutionBudget>into the interpreter and pattern VM, adding enforcement for recursion/import/execute-file depth, pattern step/state caps, response bytes, and WebSocket queue/connection bounding. - Extends config + docs + tests to support and validate the new budget keys and end-to-end enforcement (notably deep recursion and source-size refusal).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/execution_budget_test.rs | Integration coverage for new .wflcfg budget keys and end-to-end enforcement (recursion + source-size). |
| src/pattern/vm.rs | Replaces fixed VM step cap with shared budget checks; adds cancellation/state fan-out limiting and budget sharing for lookarounds. |
| src/pattern/mod.rs | Adds pattern error variants and *_with_budget APIs so interpreter pattern ops share the run budget. |
| src/main.rs | Runs runtime on a large-stack thread; enforces max_source_size before lex/parse. |
| src/lib.rs | Exposes new exec module from the crate root. |
| src/interpreter/mod.rs | Replaces timeout/opcount fields with shared ExecutionBudget; adds enforcement across recursion/import/execute-file/web response/WS limits. |
| src/interpreter/error.rs | Adds ErrorKind::ResourceLimit and display prefixing. |
| src/exec/mod.rs | Introduces exec module and re-exports budget types. |
| src/exec/budget.rs | New core implementation for shared limits/accounting and unit tests. |
| src/config.rs | Adds new config keys + parsing/validation helpers feeding the budget. |
| Docs/reference/configuration-reference.md | Documents new keys and the execution-budget concept. |
| Docs/04-advanced-features/web-servers.md | Documents response-size and WebSocket queue/connection limits as part of the shared budget. |
| Dev diary/2026-07-12-shared-execution-budget.md | Dev diary entry describing the motivation, design, and follow-ups for the shared budget. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // The step ceiling and any deadline both surface as the historic | ||
| // step-limit error so existing ReDoS handling keeps matching. | ||
| _ => PatternError::StepLimitExceeded, |
| /// A standalone VM whose pattern ceilings match the historic ReDoS defaults | ||
| /// (`MAX_STEPS` = 100_000, plus an active-state cap). Used by | ||
| /// [`super::CompiledPattern`]'s convenience methods and stdlib pattern | ||
| /// builtins, which have no interpreter budget to share. |
| // Enforce the shared source-size ceiling before doing any lexing/parsing. | ||
| // Uses the same ExecutionBudget the interpreter will run under, so the | ||
| // `max_source_size` .wflcfg knob governs every entry point (run, lint, | ||
| // analyze, dump) from one place. |
| | `web_socket_queue_bound` | integer ≥ 1 | `1024` | Max queued frames/events per WebSocket channel before shedding | | ||
| | `web_socket_max_connections` | integer ≥ 1 | `1024` | Max simultaneous live WebSocket connections | | ||
|
|
||
| ### Execution budget (resource limits) |
| let mut next_states = Vec::new(); | ||
|
|
| let mut next_states = Vec::new(); | ||
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fee3375b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Err(exceeded) = | ||
| wfl::exec::budget::ExecutionBudget::from_config(&config).check_source_bytes(input.len()) |
There was a problem hiding this comment.
With max_source_size set, this check runs only after fs::read_to_string has already read the entire top-level file into memory, and rg check_source_bytes shows no equivalent check in the load module, include, or execute file read paths. In those scenarios, a huge main file can still exhaust memory before the limit fires, and a tiny main file can parse an oversized nested WFL source without tripping the new budget at all. Load the config before reading and enforce the byte cap for every WFL source before or during the read.
Useful? React with 👍 / 👎.
| op_count: Cell::new(0), | ||
| started: Instant::now(), | ||
| max_duration: Duration::from_secs(config.timeout_seconds), | ||
| budget: Arc::new(ExecutionBudget::from_config(&config)), |
There was a problem hiding this comment.
When an execute file statement runs, it later creates the nested interpreter with Interpreter::with_config(Arc::clone(&self.config)), and this constructor always installs a fresh ExecutionBudget here. In that scenario the operation counter, deadline start time, and cancellation state reset for each executed file, so code can split work across execute file calls and exceed the configured run budget without being stopped. Add a nested-execution path that clones the parent Arc<ExecutionBudget> instead of rebuilding it from config.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main.rs (1)
766-780: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSource-size ceiling is checked after the whole file is already read into memory.
fs::read_to_string(line 766) loads the entire file beforecheck_source_bytes(774-779) ever runs, so amax_source_sizeset specifically to bound memory/IO cost doesn't actually prevent an oversized file from being fully read first — it only rejects afterward. Check the file's on-disk size viafs::metadatabefore reading.🛡️ Proposed fix: check size before reading the file
- let input = fs::read_to_string(&file_path)?; let script_dir = Path::new(&file_path).parent().unwrap_or(Path::new(".")); let config = config::load_config(script_dir); - // Enforce the shared source-size ceiling before doing any lexing/parsing. + // Enforce the shared source-size ceiling using the file's on-disk size, + // before reading its contents into memory. // Uses the same ExecutionBudget the interpreter will run under, so the // `max_source_size` .wflcfg knob governs every entry point (run, lint, // analyze, dump) from one place. + let source_len = fs::metadata(&file_path)?.len() as usize; if let Err(exceeded) = - wfl::exec::budget::ExecutionBudget::from_config(&config).check_source_bytes(input.len()) + wfl::exec::budget::ExecutionBudget::from_config(&config).check_source_bytes(source_len) { eprintln!("Error: {}", exceeded.message()); process::exit(2); } + + let input = fs::read_to_string(&file_path)?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.rs` around lines 766 - 780, Update the file-loading flow around `fs::read_to_string` to call `fs::metadata` first, compare the file’s on-disk length against the configured `ExecutionBudget` source limit, and reject oversized files before reading them. Preserve the existing error message and exit behavior, then read the file only after the preflight size check succeeds.
🧹 Nitpick comments (3)
src/config.rs (1)
767-780: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
web_server_max_response_sizecan reuseset_positive_usize.This match arm duplicates the logic of the
set_positive_usizehelper defined just above. It could be replaced with a one-liner call, consistent with the other budget keys at lines 797–838.♻️ Proposed refactor
- "web_server_max_response_size" => match value.parse::<usize>() { - Ok(0) | Err(_) => log::warn!( - "Invalid web_server_max_response_size '{}' in {}: expected a positive integer", - value, - file.display() - ), - Ok(size) => { - config.web_server_max_response_size = size; - log::debug!( - "Loaded web_server_max_response_size: {size} from {}", - file.display() - ); - } - }, + "web_server_max_response_size" => set_positive_usize( + &mut config.web_server_max_response_size, + "web_server_max_response_size", + value, + file, + ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/config.rs` around lines 767 - 780, Replace the duplicated parsing and validation match for web_server_max_response_size with a call to the existing set_positive_usize helper, passing the parsed value, configuration field, key name, and file context consistently with the neighboring budget-key arms. Preserve the current positive-integer validation and assignment behavior.src/exec/budget.rs (1)
806-818: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider testing more
from_configfield mappings.
from_config_maps_existing_keysonly asserts 3 of the 14BudgetLimitsfields. The new budget-specific fields (max_call_depth,max_import_depth,max_pattern_steps,max_source_bytes,max_response_bytes,max_ws_queue,max_ws_connections,max_operations) are simple field-to-field copies, but a regression infrom_configcould silently pass this test.♻️ Proposed additional assertions
#[test] fn from_config_maps_existing_keys() { let config = WflConfig { timeout_seconds: 42, web_server_max_body_size: 4096, web_server_request_queue_bound: 7, + max_operations: Some(99), + max_call_depth: 500, + max_import_depth: 30, + max_execute_file_depth: 2, + max_pattern_steps: 50_000, + max_pattern_states: 5_000, + max_source_size: 1024, + web_server_max_response_size: 2048, + web_socket_queue_bound: 64, + web_socket_max_connections: 32, ..Default::default() }; let limits = BudgetLimits::from_config(&config); assert_eq!(limits.max_duration, Some(Duration::from_secs(42))); assert_eq!(limits.max_request_body_bytes, 4096); assert_eq!(limits.max_pending_requests, 7); + assert_eq!(limits.max_operations, Some(99)); + assert_eq!(limits.max_call_depth, 500); + assert_eq!(limits.max_import_depth, 30); + assert_eq!(limits.max_execute_file_depth, 2); + assert_eq!(limits.max_pattern_steps, 50_000); + assert_eq!(limits.max_pattern_states, 5_000); + assert_eq!(limits.max_source_bytes, 1024); + assert_eq!(limits.max_response_bytes, 2048); + assert_eq!(limits.max_ws_queue, 64); + assert_eq!(limits.max_ws_connections, 32); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/exec/budget.rs` around lines 806 - 818, Expand the existing from_config_maps_existing_keys test to configure and assert the eight untested budget-specific mappings: max_call_depth, max_import_depth, max_pattern_steps, max_source_bytes, max_response_bytes, max_ws_queue, max_ws_connections, and max_operations. Verify each resulting BudgetLimits field matches its corresponding WflConfig value while preserving the existing assertions.tests/execution_budget_test.rs (1)
28-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider bounding
run_with_cfg's subprocess with a timeout.
Command::output()has no timeout, so if budget enforcement ever regresses into a real hang instead of a fast depth/size error — exactly the class of bug these tests exist to catch — the test run blocks indefinitely instead of failing fast.Want me to wire in a bounded wait (e.g. via the
wait-timeoutcrate) around the spawnedwflprocess inrun_with_cfg?Also applies to: 112-166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/execution_budget_test.rs` around lines 28 - 40, Update the test helper run_with_cfg to spawn the wfl process and enforce a finite timeout while waiting, rather than calling Command::output() without a bound. On timeout, terminate the child and fail the test clearly; preserve the existing configuration, script setup, and captured Output behavior for processes that finish normally, including the additional call sites noted in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/exec/budget.rs`:
- Around line 340-415: Replace the `if let ... && ...` let-chain syntax in
`check_deadline` and `charge_operation` with Rust 1.85-compatible nested
conditionals, preserving the existing deadline, operation-limit, and
cancellation behavior.
In `@src/interpreter/mod.rs`:
- Around line 2194-2209: Update budget_error so it does not clear call_stack for
catchable recursion-limit ResourceLimit errors, preserving active frames when
the error is handled by try/when. Retain stack clearing for non-catchable budget
failures and keep the existing in_count_loop cleanup and error-kind behavior
unchanged.
---
Outside diff comments:
In `@src/main.rs`:
- Around line 766-780: Update the file-loading flow around `fs::read_to_string`
to call `fs::metadata` first, compare the file’s on-disk length against the
configured `ExecutionBudget` source limit, and reject oversized files before
reading them. Preserve the existing error message and exit behavior, then read
the file only after the preflight size check succeeds.
---
Nitpick comments:
In `@src/config.rs`:
- Around line 767-780: Replace the duplicated parsing and validation match for
web_server_max_response_size with a call to the existing set_positive_usize
helper, passing the parsed value, configuration field, key name, and file
context consistently with the neighboring budget-key arms. Preserve the current
positive-integer validation and assignment behavior.
In `@src/exec/budget.rs`:
- Around line 806-818: Expand the existing from_config_maps_existing_keys test
to configure and assert the eight untested budget-specific mappings:
max_call_depth, max_import_depth, max_pattern_steps, max_source_bytes,
max_response_bytes, max_ws_queue, max_ws_connections, and max_operations. Verify
each resulting BudgetLimits field matches its corresponding WflConfig value
while preserving the existing assertions.
In `@tests/execution_budget_test.rs`:
- Around line 28-40: Update the test helper run_with_cfg to spawn the wfl
process and enforce a finite timeout while waiting, rather than calling
Command::output() without a bound. On timeout, terminate the child and fail the
test clearly; preserve the existing configuration, script setup, and captured
Output behavior for processes that finish normally, including the additional
call sites noted in the comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0686da01-1a41-43f5-9fea-b827d19c0516
📒 Files selected for processing (13)
Dev diary/2026-07-12-shared-execution-budget.mdDocs/04-advanced-features/web-servers.mdDocs/reference/configuration-reference.mdsrc/config.rssrc/exec/budget.rssrc/exec/mod.rssrc/interpreter/error.rssrc/interpreter/mod.rssrc/lib.rssrc/main.rssrc/pattern/mod.rssrc/pattern/vm.rstests/execution_budget_test.rs
…s, catchable-error stack Follow-up fixes from automated PR review on #609: - Share the ExecutionBudget with the `execute file` child interpreter (clone the parent Arc) so the deadline, operation ceiling, and cancellation span the whole run instead of resetting per executed file. - Enforce max_source_size on nested sources (load module / include / execute file) via file metadata *before* reading; the CLI top-level check now also reads metadata first, so oversized sources are refused without allocating. - budget_error only force-clears the call stack for the terminal deadline; a catchable ResourceLimit (e.g. the recursion ceiling) leaves the stack for call_function to unwind, so the depth counter stays correct after a caught recursion error under try/when. - Pattern VM checks the active-state ceiling after each expansion (fail fast) rather than only once the next generation is fully built. - Doc/comment accuracy: PatternVM::new no longer references the removed MAX_STEPS; the VM error-map comment no longer implies deadline sampling; the CLI comment says "same config" not "same budget instance"; and the duplicate "Execution budget (resource limits)" doc heading was renamed for a unique anchor. New regression tests: nested_execute_file_source_is_size_checked and execute_file_shares_the_parent_operation_budget. Full lib suite, budget suite, execute-file suite, and 107 TestPrograms pass; fmt + clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
logbie
left a comment
logbie
left a comment
There was a problem hiding this comment.
Deep review — changes required before merge
Reviewed head 1fee3375b04511d76e2366f39a9dbed98ce6bcb0 against base d293a1dec9f23324035569f0b373e1b257535334: all 13 changed files, affected call sites, existing review threads, configuration tooling, and the HTTP/WebSocket/pattern execution paths. GitHub CI, WFL Config Lint, and the review workflow are green, but the security boundaries below are not covered by those checks.
This is submitted as a COMMENT review only because the authenticated logbie account is also the PR author. Treat the P1 findings as blocking.
P1 — blocking
-
The recursion guard is bypassable after a caught limit error.
budget_error()clearscall_stackwhile the native recursive frames remain live. Since a generalwhen errorcan catchResourceLimit, the handler can recurse again from logical depth zero, stack another full quota on the existing frames, and still reach a native stack abort. The existing thread onsrc/interpreter/mod.rs:2194-2200understates the impact. Enforcement depth needs RAII/live-frame accounting separate from diagnostic stack cleanup. -
Pattern limits do not count pattern transitions. The counter advances once per outer NFA breadth round, while
step()may execute an arbitrary instruction chain; the private negative-lookahead NFA loop performs no step, cancellation, or active-state checks; and every nested lookaround VM starts its local counter at zero. Registeredpattern_matches/pattern_find/pattern_find_alland pattern split also use the standalone default budget. A configured low limit therefore does not bound the work it claims to bound. -
HTTP request bytes and pending requests remain OOM paths. A chunked body has no
Content-Length, sobody::bytes()buffers it fully before the post-buffer check. Separately, the sharedRequestGuardis unused: each listener has its own semaphore, its permit is released immediately after enqueue, and a dequeued request can wait forever inpending_responses. Repeated requests that are dequeued but never answered grow route tasks/map entries without bound. -
There is not one budget for one run. The CLI creates and discards a temporary budget for the top-level size check, then lexes/parses/analyzes before the real interpreter budget starts.
execute fileconstructs another fresh budget. The interpreter budget is private and has no cancellation handle or caller-supplied constructor; repository usage ofcancel()is limited to a unit test. This contradicts the stated parsing, deadline, cancellation, and child-execution coverage. -
Source limits still do not protect source loading. The top-level file is fully allocated before its check, and
load module,include,execute file, and the REPL do not applymax_source_sizebefore lexing. The existing source-limit thread is valid; use one bounded source loader for every entry point.
P2 — must resolve or explicitly narrow the PR contract
- The response cap clones/formats the entire payload before checking it and can leave an oversized request pending when the error is caught.
- The configuration checker/fixer does not know any new budget keys;
--configFixcomments them out as unknown, potentially revertingmax_operationsto unlimited. - Bounded WebSocket data queues now carry non-droppable lifecycle/control traffic. A full queue can lose Connect/Disconnect, and
close servercan drop its Close frame while reporting success. - The 1 GiB stack is reserved before argument routing, so even
--help/config/package commands can fail under a smaller address-space limit. It also does not protect publicInterpreterusers on ordinary threads, despite the default depth of 1000 and the PR's own note that an 8 MiB debug stack overflows near depth 40. - Resolve the open pattern error-propagation/deadline/state-allocation threads and either restore the documented Rust 1.85 compatibility or raise/test the MSRV.
Tests needed before merge
Add adversarial tests for: catch-and-recurse after the call-depth error; negative/nested lookaround under limits of 1; every WFL pattern entry point using configured limits; chunked oversized bodies; multiple listeners plus dequeued/unanswered requests; nested/REPL source limits; cancellation through an active child; oversized response cleanup; full WebSocket queues during disconnect/server close; and config check/fix round-tripping every new key.
I could not rerun Rust locally because this review environment has no cargo or rustc; git diff --check passed, and the GitHub workflow results on this exact head are green.
| ); | ||
| } | ||
| }, | ||
| "max_operations" => match value.parse::<u64>() { |
There was a problem hiding this comment.
[P2] Register every new key with ConfigChecker. parse_config_text accepts the new budget settings, but src/wfl_config/checker.rs::ConfigChecker::new still has no entries for them. --configCheck therefore reports valid settings as unknown, and --configFix comments those lines out; for max_operations, that silently changes a configured ceiling back to unlimited. Add the exact types/ranges/defaults to expected_settings and a check/fix round-trip test covering all new keys.
| return Err(PatternError::StepLimitExceeded); | ||
| } | ||
| self.budget | ||
| .check_pattern_steps(self.step_count) |
There was a problem hiding this comment.
[P1] Count actual transitions with one meter shared by nested matching. This checkpoint runs once per outer states wave, not once per VM transition: step() can execute an arbitrary Char/Literal/Jump/capture chain before returning (an epsilon Jump cycle never returns), and the negative-lookahead loop at 610–648 repeatedly steps and extends states without any budget/cancellation check. Nested lookaround VMs also reset step_count to zero, so sharing this Arc shares only the limit, not consumption. Charge inside instruction dispatch and before every fan-out, and keep the counter shared for the whole match including lookarounds.
| // (Phase 0, PR-0c) so a flood of accepted-but-unhandled requests | ||
| // sheds with 503 instead of growing memory without bound. | ||
| let queue_bound = self.config.web_server_request_queue_bound.max(1); | ||
| let queue_bound = self.budget.max_pending_requests(); |
There was a problem hiding this comment.
[P1] Enforce this as a shared accepted-but-unfinished request cap. Each listen still creates its own queue/semaphore, the permit is dropped immediately after enqueue, and ExecutionBudget::try_acquire_request() is never used outside unit tests. If WFL dequeues requests but does not respond, each route task waits forever and each sender remains in pending_responses, while the queue keeps admitting more work. Acquire a shared budget guard before body buffering and retain it until response, disconnect, timeout, channel failure, or shutdown.
| // (default 1 MB). | ||
| let request_sender_clone = request_sender.clone(); | ||
| let max_body_size = self.config.web_server_max_body_size; | ||
| let max_body_size = self.budget.max_request_body_bytes(); |
There was a problem hiding this comment.
[P1] Apply the body limit while reading, not after body::bytes(). The early check only sees an optional Content-Length; a chunked client supplies none, so Warp buffers the complete attacker-controlled body before the later body.len() check. That can OOM despite a tiny configured cap. Consume the body stream with a running byte count and abort as soon as this budget limit is exceeded.
| // Enforce the shared response-body ceiling before handing the | ||
| // payload to the transport, so a handler cannot emit an | ||
| // unbounded response. | ||
| if let Err(exceeded) = self.budget.check_response_bytes(content_bytes.len()) { |
There was a problem hiding this comment.
[P2] Check before cloning/formatting, and complete or release the pending request on failure. content_bytes has already duplicated the entire Text/Binary (or formatted an arbitrarily large object) before this check, so the cap does not bound peak allocation. Returning here also happens before pending_responses.remove; if WFL catches the resource error and continues, the client and sender can remain pending indefinitely. Validate borrowed lengths before allocation and make request completion/cleanup RAII-backed on every error path.
| for id in ids { | ||
| if let Some(tx) = map.remove(&id) { | ||
| let _ = tx.send(WsOutbound::Close); | ||
| let _ = tx.try_send(WsOutbound::Close); |
There was a problem hiding this comment.
[P2] Do not put the non-droppable Close control message on the saturated data queue. If try_send returns Full, this code discards Close and removes the only sender from the registry anyway. The writer drains and exits when the channel closes, but the reader task can remain alive waiting on the peer, retaining the connection guard after close server reported success. Use a separate cancellation/control path (or retain/abort the per-connection task) so server close is guaranteed.
| /// turn runaway recursion into a clean, catchable error instead of the OS | ||
| /// killing the whole process with a stack overflow. 1 GiB is reserved | ||
| /// virtually and committed lazily, so normal programs pay nothing for it. | ||
| const INTERPRETER_STACK_SIZE: usize = 1024 * 1024 * 1024; |
There was a problem hiding this comment.
[P2] Avoid making a 1 GiB reservation a prerequisite for every CLI operation. This thread is spawned before arguments are inspected, so even wfl --help, config commands, and package commands fail to start under an address-space limit below 1 GiB or in a 32-bit process. The workaround also does not protect public Interpreter callers on normal stacks, where the documented default depth 1000 is far above the PR's stated ~40-frame debug overflow point. Defer any special runner until actual interpretation and make recursion safety independent of a caller's stack reservation.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/interpreter/mod.rs:282
- The writer task only sends a close frame when it receives
WsOutbound::Close. If the bounded channel is full during shutdown,try_send(WsOutbound::Close)can fail and dropping the sender will just end the writer loop without sending a close frame. Consider best-effort sending a close when the channel closes (recv returnsNone) to preserve the previous shutdown semantics.
// Writer task: drains queued frames to the socket until the channel closes
// or the peer goes away.
let writer = tokio::spawn(async move {
while let Some(out) = out_rx.recv().await {
match out {
WsOutbound::Text(text) => {
if ws_tx.send(warp::ws::Message::text(text)).await.is_err() {
break;
}
}
WsOutbound::Close => {
let _ = ws_tx.send(warp::ws::Message::close()).await;
let _ = ws_tx.flush().await;
break;
}
}
}
});
| fs::write( | ||
| &main, | ||
| format!( | ||
| "execute file at \"{}\" and read output as out\n", | ||
| big.display() | ||
| ), | ||
| ) |
| let combined = run( | ||
| "program.wfl", | ||
| format!( | ||
| "{loop_body}execute file at \"{}\" and read output as out\ndisplay out\n", | ||
| child.display() | ||
| ), | ||
| ); |
| if let Ok(meta) = fs::metadata(&file_path) | ||
| && let Err(exceeded) = source_budget.check_source_bytes(meta.len() as usize) | ||
| { | ||
| eprintln!("Error: {}", exceeded.message()); | ||
| process::exit(2); | ||
| } |
| if let Ok(meta) = tokio::fs::metadata(path).await | ||
| && let Err(exceeded) = self.budget.check_source_bytes(meta.len() as usize) | ||
| { | ||
| return Err(self.budget_error(exceeded, line, column)); | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Docs/reference/configuration-reference.md (1)
530-536: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign queue-overflow documentation with actual frame handling.
Line 6692 and Line 6730 discard failed outbound
try_sendcalls without logging, so full frame queues do not emit the documented warning. Either logFullconsistently or remove that promise.As per coding guidelines, “Keep user documentation current and ship documentation with user-facing feature or behavior changes.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Docs/reference/configuration-reference.md` around lines 530 - 536, Align the web_socket_queue_bound documentation with the outbound frame handling by either adding warning logs for failed try_send calls when the queue is full, or removing the documented warning promise. Update the relevant outbound frame-send logic and the configuration reference consistently, preserving the existing queue-bound behavior.Source: Coding guidelines
tests/execution_budget_test.rs (1)
112-146: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a catch-and-continue recursion-limit regression.
These tests cover uncaught depth errors only. Add a
try/whencase that catches a resource-limit recursion error and then recurses again, covering the call-stack preservation fixed inbudget_error.As per coding guidelines, “TDD is mandatory: write a failing test before implementing any feature or bug fix.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/execution_budget_test.rs` around lines 112 - 146, Extend the execution-budget tests with a catch-and-continue case that uses try/when to catch the recursion resource-limit error, then recursively invokes the program again and verifies execution continues without stack corruption. Anchor the regression test near deep_recursion_is_a_clean_error_not_a_stack_overflow and configured_call_depth_is_honored, and make it fail before the corresponding budget_error fix is implemented.Source: Coding guidelines
src/interpreter/mod.rs (1)
8705-8707: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPropagate pattern budget exhaustion instead of collapsing it into a miss.
src/interpreter/mod.rs:8705-8707, 8744-8745The budget-aware pattern helpers still hidePatternErrorbehindbool/Option, so step/state exhaustion is indistinguishable from a normal non-match. Return aResulthere and route the error throughbudget_errorso pattern limits remain catchable at runtime.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/interpreter/mod.rs` around lines 8705 - 8707, Update the pattern-matching flow around compiled_pattern.matches_with_budget and the corresponding logic near the later match handling to preserve PatternError as a Result instead of converting exhaustion into false or None. Propagate failures through budget_error so step/state limits remain catchable at runtime, while retaining normal non-match behavior for successful evaluations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/interpreter/mod.rs`:
- Around line 2220-2235: The enforce_source_size check only validates metadata
and does not bound the subsequent read. Update the source-loading paths that use
enforce_source_size to read at most max_source_size plus one byte, then raise
budget_error when the read exceeds max_source_size; preserve the existing line
and column context and avoid relying solely on metadata, including for special
files.
---
Outside diff comments:
In `@Docs/reference/configuration-reference.md`:
- Around line 530-536: Align the web_socket_queue_bound documentation with the
outbound frame handling by either adding warning logs for failed try_send calls
when the queue is full, or removing the documented warning promise. Update the
relevant outbound frame-send logic and the configuration reference consistently,
preserving the existing queue-bound behavior.
In `@src/interpreter/mod.rs`:
- Around line 8705-8707: Update the pattern-matching flow around
compiled_pattern.matches_with_budget and the corresponding logic near the later
match handling to preserve PatternError as a Result instead of converting
exhaustion into false or None. Propagate failures through budget_error so
step/state limits remain catchable at runtime, while retaining normal non-match
behavior for successful evaluations.
In `@tests/execution_budget_test.rs`:
- Around line 112-146: Extend the execution-budget tests with a
catch-and-continue case that uses try/when to catch the recursion resource-limit
error, then recursively invokes the program again and verifies execution
continues without stack corruption. Anchor the regression test near
deep_recursion_is_a_clean_error_not_a_stack_overflow and
configured_call_depth_is_honored, and make it fail before the corresponding
budget_error fix is implemented.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3647a3fa-2eb5-45d0-955d-1146a1fd190f
📒 Files selected for processing (6)
Dev diary/2026-07-12-shared-execution-budget.mdDocs/reference/configuration-reference.mdsrc/interpreter/mod.rssrc/main.rssrc/pattern/vm.rstests/execution_budget_test.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- Dev diary/2026-07-12-shared-execution-budget.md
- src/main.rs
- src/pattern/vm.rs
…ows CI) WFL treats backslash as an escape character in string literals, so embedding a Windows path (from Path::display()) into an `execute file at "..."` statement broke parsing on windows-latest. Normalize embedded paths to forward slashes, which the runtime accepts on every platform. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
| // 4. Read file content (source-size ceiling first) | ||
| self.enforce_source_size(&resolved_path, *line, *column) | ||
| .await?; | ||
| let content = tokio::fs::read_to_string(&resolved_path) | ||
| .await |
| // 4. Read file content (source-size ceiling first) | ||
| self.enforce_source_size(&resolved_path, *line, *column) | ||
| .await?; | ||
| let content = tokio::fs::read_to_string(&resolved_path) | ||
| .await |
| self.enforce_source_size(&resolved_path, *line, *column) | ||
| .await?; | ||
| let content = tokio::fs::read_to_string(&resolved_path) | ||
| .await | ||
| .map_err(map_io_error)?; |
| // `fetch_add` returns the previous value; use it as this op's index so | ||
| // the very first op (index 0) is a sample point, matching the old code. | ||
| let index = self.operations.fetch_add(1, Ordering::Relaxed); | ||
| let sample = index & (CLOCK_SAMPLE_STRIDE - 1) == 0; | ||
|
|
||
| if sample && self.cancelled.load(Ordering::Relaxed) { | ||
| return Err(BudgetExceeded::Cancelled); | ||
| } | ||
|
|
||
| if !enforce_limits { | ||
| return Ok(()); | ||
| } |
…oint Addresses maintainer P1-4, P1-5 and related Copilot findings. - One ExecutionBudget per run: main.rs builds a single budget from the timeout-capped run_config up front and reuses it for the pre-parse source check AND the interpreter (new Interpreter::with_config_and_budget + Interpreter::budget() cancellation handle). Its deadline clock now covers lexing/parsing/analysis/interpretation, and execute-file already shares it. - Bounded source loader: read at most max_source_size+1 bytes (CLI, load module, include, execute file, and now the REPL), so an oversized source is refused without allocating it — even when metadata is unavailable, stale, or reports 0 (special files). Replaces the metadata-only enforce_source_size. - u64->usize source length converted safely (overflow = too large). - charge_operation(false) (main-loop exemption) no longer increments the operation counter, so exempt work can't trigger a post-loop Operations breach; cancellation is still honored every call. - Large interpreter stack is no longer a hard prerequisite: --help/--version run on the normal stack, and a failed large-stack reservation (tight RLIMIT_AS / 32-bit) falls back to the default stack instead of refusing to start. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
| budget: &std::sync::Arc<ExecutionBudget>, | ||
| ) -> bool { | ||
| let mut vm = PatternVM::with_budget(std::sync::Arc::clone(budget)); | ||
| vm.execute(&self.program, text).unwrap_or(false) | ||
| } |
| fn budget_error(&self, exceeded: BudgetExceeded, line: usize, column: usize) -> RuntimeError { | ||
| if *self.in_count_loop.borrow() { | ||
| *self.in_count_loop.borrow_mut() = false; | ||
| *self.current_count.borrow_mut() = None; | ||
| } | ||
|
|
||
| let kind = match exceeded { | ||
| BudgetExceeded::Deadline { .. } => { | ||
| self.call_stack.borrow_mut().clear(); | ||
| ErrorKind::Timeout | ||
| } | ||
| _ => ErrorKind::ResourceLimit, | ||
| }; | ||
| RuntimeError::with_kind(exceeded.message(), line, column, kind) | ||
| } |
…e on catch Addresses the maintainer's P1-1 (recursion guard bypassable after a caught limit) and Copilot's count-loop-state finding. - Enforcement recursion depth now lives in a dedicated `call_depth: Cell<usize>` incremented/decremented by an RAII CallDepthGuard in call_function, separate from the diagnostic `call_stack` (which may be force-cleared). The guard restores depth on every unwind, so a caught ResourceLimit can never leave the depth under-counted and pile onto still-live native frames. - budget_error no longer mutates any interpreter state. Every budget breach is catchable by a general try/when, so the call stack, count-loop flags, and recursion depth must unwind naturally; force-clearing them (the historic timeout behavior) corrupted an enclosing count loop and under-counted depth. - interpret() resets in_count_loop/current_count/call_depth up front so an uncaught terminal breach can't leak stale state into a reused interpreter (REPL). Regression test catching_a_recursion_limit_leaves_a_consistent_interpreter: a count loop survives 3 caught recursion errors, `count` stays readable, and re-recursing stays bounded (no stack overflow). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
…dget Addresses the maintainer's P1-2 and the pattern error-swallowing findings. - Transitions are now charged per *instruction* inside step()'s dispatch loop (and per negative-lookahead iteration), so an epsilon-Jump cycle or a lookaround instruction chain is bounded — not just the outer NFA waves. - ONE shared meter (ExecutionBudget::pattern_steps, reset once per top-level op) is charged by nested lookaround/lookbehind VMs too, so their work counts against the same budget instead of resetting to zero. execute/find/find_all no longer reset; CompiledPattern's top-level methods reset once. - Budget breaches PROPAGATE instead of collapsing into a non-match: the VM's find/find_all return Result, CompiledPattern's *_with_budget return Result, and the interpreter's matches/find operators plus the stdlib pattern builtins (pattern_matches/find/find_all/split) surface a catchable ResourceLimit error. - A thread-local "current budget" (set for the run in interpret()) lets the stdlib pattern builtins — whose native signature has no budget parameter — honour the run's configured max_pattern_steps/max_pattern_states. - max_pattern_steps default raised to 5_000_000 to suit per-instruction granularity (was a per-wave 100_000); ordinary patterns clear it easily. - Active-state fan-out is checked after each expansion, including in the negative-lookahead loop. Tests: pattern_step_limit_is_enforced_and_propagated / _is_catchable / patterns_run_normally_under_default_budget; budget unit tests for the shared meter and the current-budget scope guard. 107 TestPrograms + full lib pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
- ConfigChecker::new now knows every ExecutionBudget key (plus the previously unregistered web_server_request_queue_bound), so `--configCheck` accepts them and `--configFix` no longer strips them — which for max_operations would have silently reverted a configured ceiling back to unlimited. Round-trip test test_budget_keys_are_known_and_survive_fix covers all new keys. - Record the real MSRV: the codebase uses `let`-chains (stable in Rust 1.88), so Cargo.toml gains rust-version = "1.88" and CLAUDE.md's stale "1.75+" is corrected, resolving the reviewer's MSRV-vs-let-chains concern. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/repl.rs (1)
40-42: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftReset the REPL timeout per command
src/repl.rs:40-42keeps oneInterpreter/ExecutionBudgetalive for the whole session, sotimeout_secondsstarts at REPL startup. After that wall-clock window expires, later commands can tripBudgetExceeded::Deadlineon their first sampled op. Give each command a fresh budget, or make the REPL use an unlimited budget.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/repl.rs` around lines 40 - 42, Update the REPL command execution flow around new and its shared Interpreter so each command receives a fresh timeout budget based on WflConfig.timeout_seconds, or configure the REPL interpreter with an unlimited budget. Ensure later commands are not rejected because the session-start deadline expired.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/repl.rs`:
- Around line 40-42: Update the REPL command execution flow around new and its
shared Interpreter so each command receives a fresh timeout budget based on
WflConfig.timeout_seconds, or configure the REPL interpreter with an unlimited
budget. Ensure later commands are not rejected because the session-start
deadline expired.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a0be47c-013b-4ffc-99a1-a4ba23c01bbd
📒 Files selected for processing (9)
src/config.rssrc/exec/budget.rssrc/interpreter/mod.rssrc/main.rssrc/pattern/mod.rssrc/pattern/vm.rssrc/repl.rssrc/stdlib/pattern.rstests/execution_budget_test.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/config.rs
- src/pattern/mod.rs
|
CI status: now fully green — no maintainer action needed on the earlier red run. Logging the diagnosis here so it's on the record. The previous run (29184686667, sha Root cause was platform-specific, not a bug in the budget itself: both tests embedded a You've already fixed it in Worth keeping the This PR is green and awaiting review. Posted by the WFL repo warden (automated triage pass). |
… WS close Addresses maintainer P1-3 and the remaining P2 web/REPL findings. - Streaming body limit: the request-body ceiling is enforced while the body streams in (warp::body::stream + read_body_capped), so a chunked body with no Content-Length is bounded too — an oversized body gets 413 without being fully buffered. The Content-Length fast-path 413 stays for well-behaved clients. - Global in-flight cap: admission now uses the shared ExecutionBudget RequestGuard, so the accepted-request cap is enforced across all listeners (not per-server). The guard is acquired before the body is read and held until the handler responds, the client disconnects, or the request times out. - Per-request timeout: a dequeued request that is never answered is shed with 504 after web_server_response_timeout_seconds (default 300, 0 disables), freeing its slot instead of pinning it forever. - Response cap checks the borrowed Text/Binary length before duplicating into content_bytes (bounds peak allocation), then re-checks materialized fallbacks. - WebSocket close reliability: the writer sends a best-effort close frame on any exit — explicit Close, empty channel (close server dropped the sender), or a Full queue that couldn't carry Close — so `close server` always terminates the socket. Outbound send/broadcast now log on a full/closed queue (matching docs). - REPL uses a no-deadline budget so later commands aren't rejected by a session-start timeout (each other ceiling still applies; Ctrl-C interrupts). - New web_server_response_timeout_seconds config key (registered with ConfigChecker); docs updated. Full lib + web + 107 TestPrograms pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
|
Addressed CodeRabbit's two "outside diff" Major findings on the front-end checkpoints in
Both polls stay exemption-aware; since Two library-level regression tests isolate the new behavior (parse a 64-statement nested Validated: Generated by Claude Code |
logbie
left a comment
logbie
left a comment
There was a problem hiding this comment.
Fourth pass on Claude's round-3/CodeRabbit update at 4e9ad3c.
I verified the substantial fixes: main-loop depth is now RAII/nestable and inherited by execute-file; CPU-only main loops yield; inbound WebSocket assembly is capped; Connect fails closed; outbound payloads are measured before cloning; listener Drop cleanup is present; the REPL pre-checks prospective input size; the documented pattern default is corrected; and analyzer/type-checker statement recursion now polls. Exact-head CI, Config Lint, and Claude Code Review are green, and git diff --check is clean.
This is still not ready to merge. Three claimed fixes remain partial (HTTP admission lifetime, fatal type-check propagation, and front-end coverage), with actionable inline comments below.
I also do not accept the two documented deferrals as closing their threads:
- The thread-local async scope remains a P1. WFL is a public library:
src/lib.rsre-exportsInterpreter, and two instances can legally bejoin!ed/spawn_localed. "The current CLI runs one" is not an enforced API invariant. The new REPL guard is another thread-local binding held across.await; task interleaving can still cross-contaminate budgets or restore stale state. Use task-local scoping or explicit propagation. - The per-step full-text conversion is not merely an optimization. Pattern input is a runtime value and is not bounded by
max_source_size(it can come from unbounded file reads, HTTP/client data, or constructed strings).max_pattern_stepsbounds transition count, not the O(text) work/allocation performed before each charge; a 1 MiB input can incur enormous repeated conversions between sampled deadline checks. Convert once before the VM loop and checkpoint before preprocessing.
The prior public-Interpreter stack-safety thread also remains unresolved: the 1 GiB stack is CLI-only/best-effort while the public default depth is still exposed on ordinary embedding stacks.
This is a COMMENT review because the authenticated account is the PR author; treat the P1s as blocking.
| response_sender: Arc::new(tokio::sync::Mutex::new(Some( | ||
| response_sender, | ||
| ))), | ||
| _admission: Some(guard), |
There was a problem hiding this comment.
[P1] Keep the admission slot through response completion, not merely through dequeue. Moving the guard into WflHttpRequest fixes timed-out bodies that are still queued, but WaitForRequestStatement later moves only response_sender into pending_responses; the local request (and this guard) drops when that statement returns. A program can therefore dequeue requests repeatedly without responding, immediately reopening the global admission gate while every route task and pending sender remains unfinished until its 300s timeout. Store the guard with the pending response/completion state and release it on respond, disconnect/timeout pruning, channel failure, or shutdown. An Arc-backed permit shared by the route/queue/pending entry can cover both the pre-dequeue and post-dequeue lifetimes.
| /// caller that otherwise treats `TypeError`s as non-fatal warnings **must** | ||
| /// consult this and abort the run when it is `Some`, so a | ||
| /// deadline/cancellation/resource breach is never silently ignored. | ||
| pub fn take_budget_error(&mut self) -> Option<crate::exec::budget::BudgetExceeded> { |
There was a problem hiding this comment.
[P1] A fatal budget breach cannot be an optional side channel. The include from caller at interpreter/mod.rs:4417 still prints check_types errors as warnings and never calls take_budget_error(), so a deadline breach recorded here is followed by execution of the included program. Also, when TypeChecker::new()'s internal Analyzer::analyze fails at lines 281–293, this field is never populated, so even a caller that checks it sees None. Return a typed result that forces callers to distinguish budget failure from ordinary diagnostics (and propagate the analyzer's typed breach), or update and test every caller before calling this fixed.
| // Implementation of StmtParser trait | ||
| impl<'a> StmtParser<'a> for Parser<'a> { | ||
| fn parse_statement(&mut self) -> Result<Statement, ParseError> { | ||
| // Recursive front-end checkpoint: `parse_statement` is called for *every* |
There was a problem hiding this comment.
[P1] This still does not cover the lexer or recursive expression/token work. parse_statement is a statement-boundary poll: one huge list/map/pattern/expression remains a single statement, and the analyzer/type checker likewise recurse through its expressions without another checkpoint. The lexer has no budget call at all, so wfl --lex can process and format the entire capped input after its deadline, and the REPL cannot deliver same-task Ctrl-C while lexing. Add stride checkpoints at token consumption and inside expression/pattern visitors (with direct deadline/cancellation checks), plus a lexer strategy that can yield or run concurrently with cancellation. The new nested-statement tests do not exercise this case.
| writer.abort(); | ||
| _permit: None, | ||
| }) { | ||
| log::warn!("WebSocket event queue full; dropping disconnect event for {conn_id}: {err}"); |
There was a problem hiding this comment.
[P2] Disconnect is still a lossy control event. Failing closed when Connect cannot be queued fixes the authentication-order issue, but after an admitted connection closes this try_send can still drop the only application cleanup notification. A fast connect/close can leave a queued Connect that later initializes state with no matching Disconnect, and session/resource cleanup in the handler is skipped. Reserve control capacity, use a separate guaranteed lifecycle channel, or otherwise ensure each admitted Connect has a corresponding Disconnect.
… channel A shared-budget breach recorded during type checking was an optional side channel (`take_budget_error`): the `include from` caller printed the type errors as non-fatal warnings and then executed the included program, so a deadline / cancellation / resource breach hit while checking an included file was silently followed by running it. And when `TypeChecker::new()`'s internal `Analyzer::analyze` failed, the breach was rendered as ordinary `TypeError`s while the budget channel stayed empty — so even a caller that checked the side channel saw `None`. Make the distinction impossible to miss at the type level: `check_types` now returns `Result<(), TypeCheckError>` where `TypeCheckError::Budget` (fatal — stop the run) is separate from `TypeCheckError::Types` (ordinary diagnostics). The analyzer records its breach on a typed `budget_error` latch (`take_budget_error`), which `check_types` propagates as the fatal variant, closing the analysis-phase gap. Every caller now distinguishes the two: - `include from`: a budget breach is fatal (return the catchable resource/timeout error) instead of a printed warning; ordinary type diagnostics stay non-fatal warnings as before. - `load module`, main CLI, REPL: budget breach aborts the run; type diagnostics keep their existing handling. - LSP (report-only) renders either via `into_diagnostics()`. Tests: analysis-phase and type-check-phase breaches now assert the fatal `TypeCheckError::Budget` variant; existing type-checker/LSP call sites updated to the typed result. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
`PatternVM::step` re-ran `text.chars().collect()` on every transition, so a single match against a large runtime input performed O(text) allocation and copying per step. Pattern input is a runtime value that is NOT bounded by `max_source_size` (it can be an unbounded file read, HTTP/client data, or a constructed string), and `max_pattern_steps` bounds the transition count, not the O(text) work done before each charge — so a ~1 MiB input could incur enormous repeated conversions between sampled deadline checks. Materialize the input into `Vec<char>` exactly once at the top of each runner (`run_execute` / `run_find` / `run_find_all`) and thread the `&[char]` slice through `execute_at_position`, `find_at_position`, and `step`. Charge the budget once *before* that collection so an already-expired deadline or cancelled run aborts without paying to materialize a large input. Capture extraction and lookbehind slicing now reuse the same slice, and the position loops bound on `chars.len()` (character length) instead of `text.len()` (byte length), which also removes redundant no-op start positions on multibyte input. Tests: matching by character index across multibyte input; and a cancelled run aborting before materializing a 1 MiB input. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
Moving the in-flight admission guard into the queued `WflHttpRequest` fixed
timed-out bodies that were still queued, but once the interpreter dequeued a
request, `WaitForRequestStatement` moved only `response_sender` into
`pending_responses` and dropped the local request (and its guard) when the
statement returned. So the global admission gate reopened at dequeue: a
handler could dequeue requests in a loop without responding, immediately
readmitting new work while every route task and pending sender stayed
unfinished until its timeout.
Carry the admission slot with the parked response for its whole post-dequeue
life. `pending_responses` now stores a `PendingResponse { sender, admission }`,
and `respond` moves the slot into the `ResponseCompletion` guard, so it is
released only when the response actually completes — on success, on the
`Drop` 500 fallback, on disconnect/timeout pruning (a dropped pruned entry
releases it), or on shutdown (the map drops). The slot now spans the full
request lifetime, dequeue through response completion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
The connect event is fail-closed, so every live connection has run (or will run) its `on websocket connect` handler, which may initialize per-connection application state. The disconnect event — the only place that state is cleaned up — used a lossy `try_send`, so a momentarily-full event queue could drop it, leaking application state and leaving a queued Connect with no matching Disconnect. Deliver the disconnect with a blocking `send().await` instead: it resolves as the interpreter drains the bounded event queue, and returns `Err` only when the server has shut down (its receiver dropped), where no disconnect handler remains to run anyway. Message events keep their lossy `try_send` (dropping a message under overload is acceptable backpressure); only the lifecycle pair is made guaranteed. Under normal load the send completes immediately, so there is no behavior change except under the full-queue race this fixes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
…ss-contaminate `Interpreter` is re-exported from the crate root and is `!Send`, so a library embedder can legally `join!` or `spawn_local` two interpreter futures on one thread. The current budget was installed in a thread-local held across every `.await` (the interpreter run and the REPL command), so when two runs interleaved on a thread the second overwrote the first's current budget and restored stale state — the runs cross-contaminated. Install the run budget in a `tokio::task_local!` instead, via a new `ExecutionBudget::scope(budget, future)` that wraps the run. Task-local state is per-future, so each interleaved run keeps seeing its own budget across awaits. `interpret` is now a thin wrapper that scopes the budget around the run body (`interpret_inner`); an `execute file` child nests its own scope. The REPL command is likewise wrapped in `scope`. `ExecutionBudget::enter` (thread-local) is retained only as a synchronous fallback for the CLI front-end (lex/parse/analyze/type-check), which runs to completion without awaiting on a single-future runtime and therefore cannot interleave. `current()` consults the task-local scope first and this fallback only when no async scope is active — and since every async run establishes a scope that shadows it, the fallback can never cross-contaminate a run. Test: two runs with distinct budgets interleaved via `join!` on a current-thread runtime each keep observing only their own budget across an `.await` — the property a thread-local violates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
…lysis The front-end checkpoints only polled the budget at statement boundaries and during recursive *statement* traversal, so one huge expression — a million-element list, a long flat operator chain — was a single statement that lexed, parsed, analyzed, and type-checked to completion after the deadline. The lexer had no budget call at all, so `wfl --lex` processed the whole (source-size-capped) input past its deadline and a same-task Ctrl-C could not interrupt lexing. Add strided/recursive checkpoints across the front end: - Lexer: every 4096 tokens, consult the run budget and stop tokenizing cooperatively on a deadline/cancellation/operation breach (it returns `Vec`, so it yields the tokens gathered so far; the truncated stream then trips the parser's checkpoint). - Parser: a strided checkpoint at every primary-expression parse (the universal operand leaf — list elements, operator-chain terms, call arguments), so a single giant expression is interruptible, returning the breach as a `ParseError`. - Analyzer / type checker: poll the budget at the top of `analyze_expression` and `infer_expression_type`, latched via `budget_error`, so a huge expression tree inside one statement is interruptible during analysis and type checking. All polls are exemption-aware and no-ops when no run budget is installed, so normal programs (default `max_operations` unlimited) are unaffected. Test: a single 6000-element list literal trips the parser checkpoint under a tiny operation cap, where the old parser ran to completion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
The interpreter's async recursion is stack-heavy (~1 MiB per WFL call), so its `max_call_depth` only turns runaway recursion into a clean, catchable error on a stack large enough to reach the limit. The 1 GiB stack was set up privately in the CLI's `main`, so a library embedder driving `Interpreter::interpret` directly on an ordinary 8 MiB thread with the default `max_call_depth` (1000) could crash the host process with a native stack overflow that no depth limit can catch. Extract the large-stack thread setup into a public `wfl::run_with_interpreter_stack(work)` helper (and a `INTERPRETER_STACK_SIZE` constant), documented as the entry point embedders should wrap their runtime + `interpret` call in — the same one the CLI now uses (so the logic is shared, not duplicated). It returns `Err` only if the large stack cannot be reserved, so a caller can fall back or lower `max_call_depth`. The `Interpreter` type gains a `# Stack safety` doc section pointing embedders at it. Test: recursion deep enough to overflow the default test-thread stack completes when driven through the helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
Document the seven fixes landed for the maintainer's fourth-pass review: typed fatal type-check result, pattern collect-once, HTTP admission lifetime, guaranteed WebSocket disconnect, task-local budget scoping, lexer/expression front-end checkpoints, and the public large-stack embedder helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
|
Addressed all of the fourth-pass review — the three still-partial fixes, both P1s I had deferred (now accepted as blocking), and the P2 — in
Each fix ships a regression test. Validated: Generated by Claude Code |
logbie
left a comment
logbie
left a comment
There was a problem hiding this comment.
Final pass on PR #609 at 5bf878e (now 5,349 additions / 5,867 changed lines).
The fifth-round work closes the prior task-local isolation, pattern preprocessing, typed type-check, post-dequeue HTTP lifetime, Disconnect delivery, expression traversal, and embedder-helper implementation issues in their intended paths. Exact-head CI, WFL Config Lint, and Claude Code Review all pass; git diff --check is clean.
It is close, but not fully clear yet. I found two P1 merge blockers in the remediation itself:
- lexer budget failure silently returns a successful program prefix;
- timed-out dequeued HTTP requests can permanently hold the entire admission cap because cleanup requires a future request that cannot be admitted.
Three smaller correctness/API residuals are also inline: analyzer entry typing, Unicode lookbehind/index semantics, and opt-in rather than default-safe embedding stacks.
I opened issue #611 as the canonical checklist with reproduction sequences and acceptance criteria. The issue is not a reason to merge past the two P1s; those should be fixed on this PR or explicitly accepted before merge. The P2s can be handled here or deliberately scheduled through #611.
This is a COMMENT review because the authenticated account is the PR author.
| while let Some(token_result) = lexer.next() { | ||
| // Every `LEX_CHECKPOINT_STRIDE` tokens, consult the run budget and stop | ||
| // tokenizing cooperatively on a deadline / cancellation / operation | ||
| // breach. The lexer returns `Vec` (no `Result`), so this returns the |
There was a problem hiding this comment.
[P1] Do not turn a budget failure into a successful partial token stream. This break discards the BudgetExceeded and returns whatever prefix was gathered. wfl --lex then writes that partial dump and exits 0; more seriously, if the prefix ends at a valid statement boundary, parsing can succeed and a sampled deadline/cancellation failure need not immediately repeat on the next charge_operation, allowing a source prefix to be analyzed or executed. Return a typed lexer result (or explicit fatal outcome) and propagate it through every production caller. At this stride, check cancellation/deadline directly as well—nesting the 4096-token stride inside charge_operation's 1024-operation sampling can postpone those checks by millions of tokens.
| // released on respond/prune/shutdown, not at dequeue. | ||
| pending_responses.insert( | ||
| request.id.clone(), | ||
| PendingResponse { |
There was a problem hiding this comment.
[P1] Closed pending entries need timeout-driven cleanup; insertion-time pruning can deadlock the admission gate. Fill the global cap with requests that WFL dequeues but never answers, then have WFL wait for one more. When all route tasks time out, their senders close but these guards remain parked. No new request can be admitted to wake recv(), so no later insertion reaches the retain above and every slot stays consumed until shutdown. Make timeout/disconnect transition a shared request lease independently of future admission (while still keeping queued bodies accounted), and test that the cap reopens after all dequeued requests time out without another request first.
| if let Some(budget) = crate::exec::budget::ExecutionBudget::current() | ||
| && let Err(exceeded) = budget.charge_operation(!budget.is_deadline_exempt()) | ||
| { | ||
| return Err(vec![SemanticError::new(exceeded.message(), 0, 0)]); |
There was a problem hiding this comment.
[P2] Record entry-point breaches on the typed channel before returning. This branch renders the breach as SemanticError but leaves self.budget_error == None. If TypeChecker::new() enters analysis with an already-exhausted/cancelled budget, its new take_budget_error() call therefore sees nothing and returns TypeCheckError::Types, violating the fatal typed contract. The added test starts below the cap and trips later in analyze_statement, so it misses this branch. Store Some(exceeded) here and add an already-exhausted/cancelled entry test.
| text_chars[start_pos..state.pos].iter().collect(); | ||
| // Create a slice of text to match against, from the | ||
| // already-materialized character slice. | ||
| let text_slice: String = chars[start_pos..state.pos].iter().collect(); |
There was a problem hiding this comment.
[P2] Keep Unicode lookbehind entirely in character indices. The refactor correctly makes MatchResult::end a character index, but the full-slice test below still compares it with text_slice.len() (UTF-8 bytes), so positive and negative lookbehind fail or invert when this slice contains multibyte characters. Compare with the slice's character length or pass &[char] through the nested runner. Add both Unicode lookbehind cases. Also fix the public CompiledPattern::find example, which slices &text[m.start..m.end] using these character offsets as byte indices and can panic.
| /// work on the current stack (accepting that deep recursion may hit the OS limit | ||
| /// before `max_call_depth`), or set a conservatively low `max_call_depth`. A | ||
| /// panic inside `work` is propagated to the caller. | ||
| pub fn run_with_interpreter_stack<F, T>(work: F) -> std::io::Result<T> |
There was a problem hiding this comment.
[P2] This helper is useful, but stack safety is still opt-in rather than a safe default. The crate still re-exports directly callable Interpreter::new/interpret with max_call_depth = 1000, while the documentation says an ordinary embedding stack can overflow near depth 40. An embedder that misses this helper can therefore crash on untrusted WFL long before the advertised guard. Track and enforce a safe public contract: a conservative direct-construction default, a primary runner that owns interpreter creation/execution, or a stack-independent design. Add an ordinary-stack test that the default public path returns a resource error instead of aborting.
…, and three residuals The fifth review round's own remediation left two P1 merge blockers plus three correctness/API residuals (issue #611). Fix all five, each with a regression test. P1 — lexer must not turn a budget breach into a successful partial stream: The strided lexer checkpoint `break`'d on a breach and returned the prefix, so `wfl --lex` dumped it and exited 0, and a prefix ending at a statement boundary could parse/execute. The loop is now a private `lex_positions_core` taking a checkpoint closure: `lex_wfl_with_positions` supplies a no-op (never truncates — LSP/tooling/tests unaffected) and the new `lex_wfl_with_positions_checked` returns `Result<_, BudgetExceeded>`. Every production execution caller (CLI run and `--lex`, nested execute-file/include/load-module, REPL) propagates it. At each stride the deadline and cancellation are checked directly (not via `charge_operation`'s sampling, which nested in the 4096-token stride could postpone them by millions of tokens); the operation ceiling is charged separately. P1 — a timed-out pending HTTP request can no longer wedge admission: The guard was parked in the interpreter's pending map and pruned only when a later dequeued request was inserted; once the cap filled with unanswered requests, no new request could be admitted to trigger the prune, pinning every slot forever. The guard now rides with the warp transport future (which outlives the dequeue while awaiting the response), so the slot is held through handling and released when that future ends — respond, response timeout, or client disconnect — independently of any future admission. The bounded request mpsc still caps queued bodies. P2 — analyzer entry-time breach preserves the typed channel: the phase-boundary checkpoint now stores `budget_error` before returning, so `TypeChecker::new()` sees a cancelled/exhausted entry breach as fatal `TypeCheckError::Budget`, not misclassified `Types`. P2 — Unicode lookbehind stays in character indices: the full-slice test compared `MatchResult::end` (chars) against `text_slice.len()` (bytes), inverting positive/negative lookbehind on multibyte input; it now compares the slice's character length. The public `CompiledPattern::find` doc example that sliced `&text[m.start..m.end]` with char offsets is fixed to extract by chars. P2 — the default public interpreter path is stack-safe: `Interpreter::new()` now caps recursion at the conservative `DEFAULT_EMBED_CALL_DEPTH` (12) instead of the config default (1000), which only stays catchable on the CLI's 1 GiB stack; `with_config`/`with_config_and_budget` still honor the configured depth so the CLI reaches 1000. A deep program on an ordinary stack now returns a catchable call-depth error instead of aborting. Tests: typed lexer outcomes (cancel/deadline/operations), no-truncation, and `--lex` exit; a live-server admission-reopens-after-timeout regression; a cancelled-budget analyzer entry test; positive/negative Unicode lookbehind over a 2-byte char; and an ordinary-8-MiB-stack embedding test. Dev Diary updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
|
Addressed all of issue #611 — both P1 blockers in the fifth-round remediation and the three residuals — in
Acceptance criteria — all met:
Validated: One judgment call worth flagging: on P2-5 I chose the conservative-default option (option 1 from the issue) — Generated by Claude Code |
logbie
left a comment
logbie
left a comment
There was a problem hiding this comment.
Sixth-round follow-up on 8bd70bb: the five #611 remediations are present and the principal fixes look correct. The HTTP guard now has transport-future lifetime, analyzer typing and Unicode indexing are corrected, the default embedding depth is conservative, and execution callers use the typed lexer path.
One residual remains in the typed lexer boundary behavior (inline). The strided checkpoint is only called after token 4096, so a short input can succeed even when its budget is already cancelled or expired. This is most directly visible in --lex, which has no later parser/analyzer checkpoint and can write a dump and exit 0 under an already-expired budget.
CI on this exact head is otherwise healthy: all completed jobs pass; only Windows Run All Integration Tests remains in progress at the time of review.
This is a COMMENT review because the authenticated account is the PR author.
| let mut last_span_end = 0; | ||
| // Strided run-budget checkpoint counter. The lexer is otherwise a tight loop | ||
| // with no budget call, so a large (source-size-capped) input would tokenize | ||
| // fully even after the run's deadline, and a same-task Ctrl-C could not |
There was a problem hiding this comment.
[P2] Check the budget at the lexer boundary, not only after a full stride. checkpoint() is first invoked when lexed_tokens == 4096, so lex_wfl_with_positions_checked("display 1\n") returns Ok even if the installed budget was already cancelled or has a zero/expired deadline. In --lex there is no later phase to catch that breach, so a short source can still produce a successful dump under an already-expired run budget. Invoke the checkpoint once before entering the loop (and preferably once after the loop to catch a breach during the final partial stride), while retaining strided checks for long inputs. Add short-input cancelled/deadline regressions.
… stride
The strided lexer checkpoint fired only at token 4096, 8192, … so a source
shorter than one stride never consulted the budget: an already cancelled or
already expired budget lexed to a successful `Ok`. This was most visible in
`--lex`, which has no later parser/analyzer checkpoint and so could dump a short
source and exit 0 under an already-breached run budget.
`lex_positions_core` now invokes the checkpoint once before the first token
(catching an already-breached budget for any input length) and once after the
last (catching a breach during the final partial stride), keeping the strided
checks for long inputs. The non-budgeted `lex_wfl_with_positions` still supplies
a no-op checkpoint, so it is unaffected.
Tests: `lex_wfl_with_positions_checked("display 1\n")` under an already-cancelled
budget and under an already-expired deadline both abort with the typed breach at
the entry checkpoint.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
|
Fixed the lexer-boundary residual in The strided checkpoint fired only at token 4096+, so a source shorter than one stride never consulted the budget — an already-cancelled/expired budget lexed to a successful Regressions added: Validated: Generated by Claude Code |
logbie
left a comment
logbie
left a comment
There was a problem hiding this comment.
Final verification on 285d2e0: no remaining findings.
The short-input lexer residual is fixed correctly: the checked core now polls at entry, at each stride, and after the final partial stride; cancelled/expired short inputs have typed regressions; and the unbudgeted lexer remains unaffected through its no-op checkpoint. I also rechecked the prior #611 remediation paths and found no new regression introduced by this delta.
Exact-head CI is fully green: CI, WFL Config Lint, and Claude Code Review all completed successfully. From this review, PR #609 is ready to merge.
This is a COMMENT review only because GitHub does not allow the authenticated PR author to approve their own pull request.
Summary
WFL enforced a dozen unrelated resource ceilings from a dozen unrelated places, and several audit-flagged resources had no ceiling at all. This PR replaces that scatter with one object —
exec::budget::ExecutionBudget— that travels with a run through parsing, evaluation, pattern matching, web handling, and module loading, and owns every ceiling as one coherent mechanism.What the budget now owns
Interpreter.max_duration+op_countthrottledebug_assert!only)execute filedepthconst MAX_EXECUTE_FILE_DEPTH = 4const MAX_STEPS = 100_000web_server_max_body_sizeweb_server_request_queue_boundDesign
src/exec/budget.rs—ExecutionBudgetisSend + Sync(atomics only), so anArc<ExecutionBudget>clones into the multi-threaded web transport (warp accept tasks, per-connection WebSocket tasks) without anyRc/RefCellcrossing a thread boundary. The interpreter core stays!Send; this shares a small atomic-only object exactly likeArc<WflConfig>already is.BudgetLimits::from_configmaps the existing.wflcfgkeys plus nine new budget keys.BudgetExceededkeeps the historic timeout wording andErrorKind::Timeoutso existing timeout handling still matches; a newErrorKind::ResourceLimitcovers the rest.op_count/started/max_durationforbudget: Arc<ExecutionBudget>;check_timecharges an operation (deadline exempt insidemain loop, cancellation always honored). Recursion, import, execute-file, and response-byte guards call the budget; request-body/queue bounds source from it; WebSocket channels are now bounded withtry_sendshed-on-Full, and connections are capped via an RAII guard.MAX_STEPSremoved; each match checks steps and (new) active states against the shared budget, plus cancellation.matches/findoperators share the run budget.mainnow runs the runtime on a dedicated 1 GiB stack. That makesmax_call_depth(default 1000) a clean, catchable "Maximum call depth exceeded" error instead of a nativeSIGABRT, and lets programs recurse deeper than before.Backward compatibility
The three pre-existing knobs (timeout, request body, request queue) keep their defaults and behavior exactly — the budget just sources their values.
max_operationsis off by default; every new ceiling defaults generously enough that no existing program trips it.Testing
src/exec/budget.rs: 11 unit tests (ceiling/exemption/cancellation/deadline, depth>=vs pattern>semantics, byte caps, RAII guards,from_config,Send + Sync).tests/execution_budget_test.rs: config parsing of all new keys + end-to-end (deep recursion → clean error, not stack overflow; configured ceiling honored; oversized source refused; shallow program still runs).test_timeout_forever_loop),web_queue_bound_test,websocket_test,execute_file_test, 107/107TestPrograms/, and the web-driver tests all pass.cargo fmt --checkandclippy --all-targets -- -D warningsare clean.Docs
Docs/reference/configuration-reference.md: the nine new keys + an "Execution budget (resource limits)" section.Docs/04-advanced-features/web-servers.md: response-size and WebSocket queue/connection limits.Dev diary/2026-07-12-shared-execution-budget.md.Follow-ups (noted in the Dev Diary)
pattern_replace/split) run under a standalone budget with the same step/state ceilings; threading the run budget through their native signatures is a future refactor.reqwestclient still has no per-request timeout/in-flight cap (a separate audit item).🤖 Generated with Claude Code
https://claude.ai/code/session_01Y498gapRYjXfGSciauq49A
Generated by Claude Code
Summary by CodeRabbit
execute file, pattern matching, bounded source loading, and coordinated HTTP/WebSocket limits.execute filedepth, pattern steps/states, and source/HTTP/WebSocket sizes (queue/connection caps included).