fix(vm): restore async-generator invariants and PromiseResolve abrupt handling (#706, #707, #708, #709) by replygirl · Pull Request #950 · trynova/nova

replygirl · 2026-02-16T06:09:14Z

Restore async-generator state invariants and PromiseResolve abrupt-completion handling in async-generator await/return paths.

This does not attempt to silence assertions; it fixes invalid transition paths and preserves spec preconditions where algorithms use Assert:.

Scope

Async-generator state-machine correctness for #706, #707, #708, #709
PromiseResolve abrupt-completion behavior where async-generator algorithms do ? PromiseResolve(%Promise%, x)
Eval behavior change (unexpected VM await/yield in script eval -> SyntaxError) retained as originally proposed

Change Grouping (for review)

State invariants and transition preconditions
PromiseResolve abrupt-completion behavior in async-generator await/return paths
Evidence mapping (issue -> codepath -> regression tests)

Issue Mapping

#706 AsyncGenerator::append_to_queue called on completed AsyncGenerators

Root cause: state/queue sequencing could place requests into completed-flow paths that later violated transition assumptions.
Fix direction: keep queue behavior but restore strict state-transition invariants so invalid transition entry points are not papered over.
Main code paths:
- nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects.rs

#707 AsyncGenerator::resume_await could enter unreachable code

Root cause: resume_await could be entered from an invalid state sequence.
Fix direction: keep spec-required state assertions and transition preconditions explicit (no silent fallback returns).
Main code paths:
- nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects.rs
- nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

#708 async_generator_resume could enter unreachable code

Root cause: AsyncGeneratorResume preconditions were not enforced by strict transition usage.
Fix direction: enforce suspended-state preconditions and keep Assert-style checks in resume/drain algorithms.
Main code paths:
- nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

#709 specification assertion failure in AsyncGenerator.prototype.return

Root cause: abrupt completion from PromiseResolve in return-await flow was not modeled correctly.
Fix direction: use fallible PromiseResolve handling in async-generator await/return flow and route abrupt completion through complete-step + drain-queue behavior.
Main code paths:
- nova_vm/src/ecmascript/builtins/promise.rs
- nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs
- nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects/async_generator_prototype.rs

Validation Matrix

Issue	Regression test	Result
#709	`tests/test262/test/built-ins/AsyncGeneratorPrototype/return/return-state-completed-broken-promise.js`	Pass (loose + strict)
#709	`tests/test262/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js`	Pass (loose + strict)
#709	`tests/test262/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedYield-broken-promise-try-catch.js`	Pass (loose + strict)
#707/#708	`tests/test262/test/staging/sm/AsyncGenerators/for-await-of-error.js`	Pass (loose + strict)
#707/#708	`tests/test262/test/staging/sm/async-functions/await-error.js`	Pass (loose + strict)

Commands Run

cargo check -p nova_cli
cargo check -p tests
cargo run -p tests --bin test262 -- eval-test built-ins/AsyncGeneratorPrototype/return/return-state-completed-broken-promise.js
cargo run -p tests --bin test262 -- eval-test built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js
cargo run -p tests --bin test262 -- eval-test built-ins/AsyncGeneratorPrototype/return/return-suspendedYield-broken-promise-try-catch.js
cargo run -p tests --bin test262 -- eval-test staging/sm/AsyncGenerators/for-await-of-error.js
cargo run -p tests --bin test262 -- eval-test staging/sm/async-functions/await-error.js

Fixes #706

Fixes #707

Fixes #708

Fixes #709

…rynova#708, trynova#709) Harden async generator state transitions and queue-drain handling to avoid unreachable/panic paths, and convert invalid eval VM await/yield completions into SyntaxError instead of crashes. Also updates Test262 expectations for the now-passing async generator and SM async crash tests. Fixes trynova#706 Fixes trynova#707 Fixes trynova#708 Fixes trynova#709

replygirl · 2026-02-16T06:10:28Z

i am out of my depth here but the tests pass 🤷‍♀️

aapoalas

Hey,

Thank you for the interest and contribution, but unfortunately this is not The Way. Removing all the assertions will get you code that does not crash, but it does not fix the actual mistakes. That the assertions fire means one of three things:

We have some related code which should be making the asserted things impossible, but due to a mistake is not doing so. That code should be fixed.
We have found an ECMAScript specification bug. That should be reported, but this is unlikely.
There was a specification bug and the specification has been updated, but we haven't updated our sources to match the fixed specification. This should be checked and discrepancies fixed.

aapoalas · 2026-02-19T19:29:25Z

...s/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

+    let generator = generator.unbind();
+    let return_value = value.unbind();
    // 7. Let promiseCompletion be Completion(PromiseResolve(%Promise%, completion.[[Value]])).
+    let constructor_error = if let Value::Promise(promise) = return_value {


issue: This stuff is pretty egregiously wrong. You're doing promise.constructor here, seeing if it throws an error and if it does then you use that error to stop the async generator... What?

aapoalas · 2026-02-22T11:10:40Z

...s/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

    let execution_context = agent.pop_execution_context().unwrap();
    let generator = scoped_generator.get(agent).bind(gc.nogc());
    generator.transition_to_awaiting(agent, vm, kind, execution_context);
+    let generator = generator.unbind();


issue: let x = x.unbind() is always wrong, see https://github.com/trynova/nova/blob/main/GARBAGE_COLLECTOR.md#never-unbind-a-value-into-a-local-variable

aapoalas · 2026-02-22T11:11:28Z

...s/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

+    };

    // 7. Perform PerformPromiseThen(promise, onFulfilled, onRejected).
    inner_promise_then(agent, promise, handler, handler, None, gc.nogc());


note: handler is now use-after-free because try_resolve can trigger GC and handler is unrooted.

aapoalas · 2026-02-22T11:13:58Z

...s/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

    // Assert: generator.[[AsyncGeneratorState]] is draining-queue.
    // 2. Let queue be generator.[[AsyncGeneratorQueue]].
-    let Some(AsyncGeneratorState::DrainingQueue(queue)) = &mut data.async_generator_state else {
+    let AsyncGeneratorState::DrainingQueue(queue) =


issue: Meaningless change.

aapoalas · 2026-02-22T11:14:36Z

nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects.rs

        let async_generator_state = &mut self.get_mut(agent).async_generator_state;
-        let (vm, execution_context, queue) = match async_generator_state.take() {
-            Some(AsyncGeneratorState::SuspendedStart {
+        let state = async_generator_state.take().unwrap();


issue: Meaningless change.

aapoalas · 2026-02-22T11:17:52Z

nova_vm/src/ecmascript/builtins/promise.rs

+    ///
+    /// This variant implements abrupt-completion behavior for internal callers
+    /// that must handle `? PromiseResolve(%Promise%, x)`.
+    pub fn try_resolve(agent: &mut Agent, x: Value, mut gc: GcScope<'a, '_>) -> JsResult<'a, Self> {


issue: try_ methods should always take NoGcScope, otherwise they're not try-methods.

aapoalas · 2026-02-22T11:20:26Z

...s/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

+    let promise = match Promise::try_resolve(agent, awaited_value, gc.reborrow()) {
+        Ok(promise) => promise.unbind().bind(gc.nogc()),
+        Err(err) => {
+            generator.resume_await(agent, PromiseReactionType::Reject, err.value().unbind(), gc);


thought: This seems suspect; I am not sure if this is correct.

aapoalas · 2026-02-22T11:21:20Z

...s/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

+        .map(|promise| promise.unbind())
+        .map_err(|err| err.unbind());
+    let promise = match promise_completion {
+        // 8. If promiseCompletion is an abrupt completion, then


praise: Based on the comment at least, this indeed seems like a missing handling. Nice catch.

aapoalas

So I'm assuming this is largely if not entirely vibe-coded. That's... fine I guess, but preferably do mention it (and the extent of it) so that I can review accordingly.

As it stands, there's too much changes in this for me to really make heads or tails of what is actually necessary and what is just LLM hallucination.

aapoalas · 2026-02-22T11:22:05Z

...s/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

    // Assert: generator.[[AsyncGeneratorState]] is draining-queue.
    // 2. Let queue be generator.[[AsyncGeneratorQueue]].
-    let Some(AsyncGeneratorState::DrainingQueue(queue)) = &mut data.async_generator_state else {
+    let AsyncGeneratorState::DrainingQueue(queue) =


issue: This is meaningless; unwrap the Option or unreachable the None – it's the same thing.

aapoalas · 2026-02-22T11:22:08Z

...s/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

            let data = generator.get_mut(agent);
            // iii. If queue is empty, then
-            let Some(AsyncGeneratorState::DrainingQueue(queue)) = &mut data.async_generator_state
+            let AsyncGeneratorState::DrainingQueue(queue) =


issue: This is meaningless; unwrap the Option or unreachable the None – it's the same thing.

aapoalas · 2026-02-22T11:22:31Z

nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects.rs

-    /// > NOTE: In our implementation, EXECUTING is split into an extra
-    /// > EXECUTING-AWAIT state. This also checks for that.
-    pub(crate) fn is_active(self, agent: &Agent) -> bool {
+    pub(crate) fn is_draining_queue(self, agent: &Agent) -> bool {


nitpick: Unnecessary code churn from moving two functions around.

aapoalas · 2026-02-22T11:24:10Z

nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects.rs

-            Some(AsyncGeneratorState::SuspendedStart {
+        let state = async_generator_state.take().unwrap();
+        let (vm, execution_context, queue) = match state {
+            AsyncGeneratorState::SuspendedStart {


thought: I'm not sure if this can happen... If it can, then this is a really nice catch and explains a lot about the failures. I'd like to see only these changes in a separate PR and see the effect that has.

aapoalas · 2026-02-22T11:24:18Z

nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects.rs

        // 1. Assert: generator.[[AsyncGeneratorState]] is either suspended-start or suspended-yield.
        let state = self.get_mut(agent).async_generator_state.take().unwrap();
        let (vm, execution_context, queue, kind) = match state {
+            AsyncGeneratorState::SuspendedStart {


note: Same as above.

aapoalas · 2026-02-22T11:25:29Z

nova_vm/src/ecmascript/builtins/global_object.rs

+                let result = match Vm::execute(agent, exe.clone(), None, gc.reborrow()) {
+                    ExecutionResult::Return(value) => Ok(value.unbind()),
+                    ExecutionResult::Throw(err) => Err(err.unbind()),
+                    ExecutionResult::Await { .. } | ExecutionResult::Yield { .. } => Err(agent


issue: This should be impossible to happen, we should get an early SyntaxError from our parser. This also does not seem to have anything to do with the async-generator work.

replygirl · 2026-02-22T11:42:17Z

So I'm assuming this is largely if not entirely vibe-coded. That's... fine I guess, but preferably do mention it (and the extent of it) so that I can review accordingly.

As it stands, there's too much changes in this for me to really make heads or tails of what is actually necessary and what is just LLM hallucination.

i'm grateful for the detailed feedback, this definitely isn't my domain, i'm going to give it one more round and if it's not close you can say so without elaboration, i want to respect your time and energy and will be happy to close the pr

aapoalas · 2026-02-22T11:44:37Z

So I'm assuming this is largely if not entirely vibe-coded. That's... fine I guess, but preferably do mention it (and the extent of it) so that I can review accordingly.
As it stands, there's too much changes in this for me to really make heads or tails of what is actually necessary and what is just LLM hallucination.

i'm grateful for the detailed feedback, this definitely isn't my domain, i'm going to give it one more round and if it's not close you can say so without elaboration, i want to respect your time and energy and will be happy to close the pr

I think splitting the PR up into smaller pieces would be nice; just add the SuspendedStart branches in one PR, add the praise-worthy missed path in another PR, and see what those do individually.

replygirl · 2026-02-22T12:18:16Z

I think splitting the PR up into smaller pieces would be nice; just add the SuspendedStart branches in one PR, add the praise-worthy missed path in another PR, and see what those do individually.

ok i broke #709 out into #958 to start, may come back around for a second on SuspendedStart when time allows

aapoalas requested changes Feb 19, 2026

View reviewed changes

replygirl changed the title ~~fix(vm): harden async-generator/eval paths (#706, #707, #708, #709)~~ fix(vm): restore async-generator invariants and PromiseResolve abrupt handling (#706, #707, #708, #709) Feb 22, 2026

replygirl added 3 commits February 22, 2026 03:07

fix(vm): restore async-generator state-machine invariants

6916636

fix(vm): handle PromiseResolve abrupt completion in async generators

879b032

fix(vm): tighten draining-queue transition state preconditions

cf52ab7

aapoalas reviewed Feb 22, 2026

View reviewed changes

aapoalas requested changes Feb 22, 2026

View reviewed changes

replygirl mentioned this pull request Feb 22, 2026

fix(vm): handle AsyncGeneratorAwaitReturn PromiseResolve abrupt completion (#709) #958

Open

replygirl closed this Feb 22, 2026

Conversation

replygirl commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Scope

Change Grouping (for review)

Issue Mapping

#706 AsyncGenerator::append_to_queue called on completed AsyncGenerators

#707 AsyncGenerator::resume_await could enter unreachable code

#708 async_generator_resume could enter unreachable code

#709 specification assertion failure in AsyncGenerator.prototype.return

Validation Matrix

Commands Run

Uh oh!

replygirl commented Feb 16, 2026

Uh oh!

aapoalas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aapoalas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

replygirl commented Feb 22, 2026

Uh oh!

aapoalas commented Feb 22, 2026

Uh oh!

replygirl commented Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

replygirl commented Feb 16, 2026 •

edited

Loading