feat(ecmascript): add execution_timeout to AgentOptions#990
Open
Foorack wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an embedder-configurable execution time limit to the Nova VM by introducing execution_timeout in AgentOptions and enforcing it via a per-execution Instant deadline checked from the bytecode dispatch loop.
Changes:
- Add
execution_timeout: Option<Duration>toAgentOptionsand track a per-runexecution_deadlineonAgent. - Set/clear the deadline around
script_evaluation, and enforce it on every bytecode instruction dispatch. - Add a regression test ensuring an infinite loop is interrupted by the configured timeout; update CLI agent construction to initialize the new option.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
nova_vm/src/engine/bytecode/vm.rs |
Enforces an active execution deadline in the bytecode instruction dispatch loop by throwing a JS Error on timeout. |
nova_vm/src/ecmascript/scripts_and_modules/script.rs |
Establishes and clears a per-execution deadline around script_evaluation; adds a timeout regression test. |
nova_vm/src/ecmascript/execution/agent.rs |
Extends AgentOptions with execution_timeout and adds execution_deadline storage on Agent. |
nova_cli/src/lib/lib.rs |
Initializes execution_timeout when constructing the CLI agent. |
nova_cli/src/lib/globals.rs |
Initializes execution_timeout for child agents created by CLI internals. |
Comments suppressed due to low confidence (1)
nova_vm/src/ecmascript/execution/agent.rs:74
AgentOptionsis a public, non-#[non_exhaustive]struct, so addingexecution_timeoutis a SemVer-breaking change for downstream users constructing it with struct literals. If 1.x compatibility is a goal, consider#[non_exhaustive]and/or a constructor/builder, and ensure the release/versioning strategy accounts for this break.
/// Wall-clock time limit for a single script execution. When set, any
/// script that runs longer than this duration will be terminated with a
/// JavaScript `Error`.
pub execution_timeout: Option<std::time::Duration>,
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // already-running deadline rather than resetting it. | ||
| let set_deadline = agent.execution_deadline.is_none(); | ||
| if set_deadline && let Some(timeout) = agent.options.execution_timeout { | ||
| agent.execution_deadline = std::time::Instant::now().checked_add(timeout); |
Comment on lines
+70
to
+72
| /// Wall-clock time limit for a single script execution. When set, any | ||
| /// script that runs longer than this duration will be terminated with a | ||
| /// JavaScript `Error`. |
Comment on lines
+901
to
+902
| /// Per-execution deadline derived from `options.execution_timeout`. | ||
| /// Set at the start of `script_evaluation` and cleared when it returns. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow embedders to set execution timeout for security.