Skip to content

feat(fiber): coroutine scope — Coroutine::spawn()/await()/concurrently() - #277

Open
roxblnfk wants to merge 9 commits into
1.xfrom
feat/fiber-coroutines
Open

feat(fiber): coroutine scope — Coroutine::spawn()/await()/concurrently()#277
roxblnfk wants to merge 9 commits into
1.xfrom
feat/fiber-coroutines

Conversation

@roxblnfk

@roxblnfk roxblnfk commented Aug 6, 2026

Copy link
Copy Markdown
Member

What was changed

Every #[RunInFiber] test now runs inside its own coroutine scope: the test body is task #0 of a per-test scheduler, and Coroutine::spawn() / ->await() / Coroutine::concurrently() add coroutines to the same schedule. Between rounds the scope relays control outward, so coroutines keep interleaving with the case's other tests. The scope sits at the new ORDER_ASYNC_COROUTINE — inside both the scoped-state guards and the coverage window — so a coroutine's assertions, messages and covered lines all belong to the test that spawned it. Supporting core change: #[FallbackInterceptor] is now repeatable, so one attribute can wire interceptors at two positions.

See commit history for details.

Why?

Concurrency was only expressible between whole tests. Racing two workers, or a client against a server, inside one test meant driving fibers by hand — with no scheduling, no result plumbing and no failure reporting.

Checklist

  • Tested
    • Tested manually
    • Unit tests added
  • Documentation

@roxblnfk
roxblnfk requested a review from a team as a code owner August 6, 2026 10:18
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.59459% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
plugin/fiber/src/Internal/Scheduler.php 94.54% 6 Missing ⚠️
plugin/fiber/src/Internal/FiberTestBatchRunner.php 80.00% 2 Missing ⚠️
plugin/fiber/src/Coroutine.php 96.29% 1 Missing ⚠️
plugin/fiber/src/Exception/CompositeException.php 80.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@roxblnfk
roxblnfk requested a lite review from Copilot August 6, 2026 17:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@roxblnfk
roxblnfk requested a balanced review from Copilot August 6, 2026 18:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (2)

tests/Core/Pipeline/CacheTest.php:49

  • Assertion argument order is inconsistent within this file (e.g., Assert::same([MockInterceptor::class], $result) vs Assert::same($result, [])). Even though equality is symmetric, inconsistent ordering makes failure output harder to interpret. Please standardize the argument order across these tests (following the project's convention) so diffs and assertion messages remain clear.
    public function resolveAliasesWithFallbackInterceptorAttribute(): void
    {
        $result = Cache::resolveAliases(AttributeWithFallback::class);

        Assert::same([MockInterceptor::class], $result);
    }

    /**
     * A repeated FallbackInterceptor attribute wires every listed interceptor, in declaration order.
     */
    public function resolveAliasesCollectsRepeatedFallbacks(): void
    {
        $result = Cache::resolveAliases(AttributeWithSeveralFallbacks::class);

        Assert::same([MockInterceptor::class, SecondMockInterceptor::class], $result);
    }

    /**
     * When resolveAliases is called with a class that has no FallbackInterceptor attribute,
     * it should return an empty list.
     */
    public function resolveAliasesWithoutFallbackInterceptorAttribute(): void
    {
        $result = Cache::resolveAliases(AttributeWithoutFallback::class);

        Assert::same($result, []);
    }

plugin/fiber/src/Internal/CoroutineScopeInterceptor.php:98

  • Status handling is hard-coded to Failed/Error when deciding whether to preserve the body's status. Since earlier logic already uses Status::isFailure(), consider reusing that here (or a dedicated 'severity' comparison) to avoid missing other failure-like statuses if they exist and to keep the logic consistent.
        if ($errors !== []) {
            // A failed body keeps its own failure as the root: chain it in front of the coroutine
            // errors so nothing is dropped, and keep the harsher of the two statuses.
            $result->failure === null or $errors = [$body->id => $result->failure] + $errors;

            $failed = $result->status === Status::Failed || $result->status === Status::Error;
            $result = $result
                ->with(status: $failed ? $result->status : Status::Error)
                ->withFailure(new CompositeException($errors));
        }

An Interceptable attribute may now wire several interceptors, each with its
own pipeline position: Cache::resolveAliases() collects every
FallbackInterceptor attribute (walking the parent chain as before), and
InterceptorProvider instantiates each resolved class with the attribute.

Needed by testo/fiber, where #[RunInFiber] wraps the test pipeline in a fiber
outside the scoped-state guards and opens the coroutine scope inside them.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Every #[RunInFiber] test now runs inside its own coroutine scope: the test
body is task #0 of a per-test scheduler, and Coroutine::spawn() adds
coroutines to the same round-robin schedule. Between rounds the scope relays
control upward with a suspend of its own, so coroutines keep interleaving
with the case's other tests under a class-level #[RunInFiber].

- Scheduler rewritten from a static one-shot into a dynamic instance: tasks
  may be spawned mid-drive, await parks a task until its target settles, and
  Scheduler::current() exposes the ambient scope to the Coroutine helpers.
- The scope is structured: pending coroutines are driven after the body
  returns; a failed body cancels them (CancelledException is thrown into
  each pending fiber); an await cycle is broken with a DeadlockException
  raised at the first parked await().
- Coroutine failures always surface wrapped in CompositeException — even a
  single one — via await()/concurrently() or at scope close for unawaited
  ones (the test is marked Error). The body's own throw stays unwrapped.
- The scope lives in the new CoroutineScopeInterceptor at
  ORDER_CLOSE_TO_TEST — inside the scoped-state guards, so coroutines are
  resumed with their test's assertion/messenger state swapped in — while
  RunInFiberInterceptor keeps the fiber wrap outside the guards; both are
  wired by the same attribute via the now-repeatable #[FallbackInterceptor].

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
The pipeline below the coroutine scope captures test throwables into the
TestResult, so a failed body settled with no error on its task and
Scheduler::drive() never saw the failure — pending coroutines were driven
to completion instead of being cancelled as documented.

drive() now takes a failure predicate for the primary task, and
CoroutineScopeInterceptor passes one that recognizes a failed result
(Status::isFailure()), so the scope tears down as the docs promise.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
An await cycle crossing schedulers — a scope's coroutine awaiting another
scope's through a shared handle — was invisible to the local deadlock
check: every scope saw its parked tasks as possibly unparkable by the
outer schedule and relayed forever, livelocking the run.

The check now walks the awaiting links themselves, which naturally cross
scheduler boundaries: a chain that runs into a cycle can never be
unparked by any schedule, so the first doomed task gets the
DeadlockException; a chain that ends outside a cycle still relays. Each
scheduler only ever throws into its own tasks, so coroutines are still
resumed exclusively from their own scope's drive frame. The deadlock
message marks foreign links as "another scope's" — task ids are
per-scheduler and would collide unqualified.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
A task spawned during the scope teardown — e.g. from a cancelled
coroutine's finally block — silently joined a schedule nobody drives
anymore: never stepped, never reported. The scheduler now marks itself
closing when it starts cancelling pending tasks and rejects further
spawns with a LogicException, which surfaces through the unwinding
coroutine's error instead of vanishing.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
A cancelled task settled with finished=true and a null error, so await()
returned null — indistinguishable from a legitimate null result for a
sibling's finally unwinding on the same cancellation. The task now
remembers it was cancelled, and await() rethrows a CancelledException
instead of forging a result.

The exception is deliberately unwrapped: cancellation is the scope's
control signal, not a failure raised by the coroutine, so the
CompositeException contract does not apply — matching how the scope
already keeps cancellations out of the surfaced errors.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
concurrently() returned results keyed by the argument keys but bundled
failures keyed by the scheduler's internal task ids — with named
arguments the error's origin was untraceable, and with positional ones
the ids looked like argument indices while being off by one (the test
body is task #0).

Each await() composite wraps exactly one task's error; concurrently()
now unwraps it and re-keys it by the argument, making $errors symmetric
to the results. CompositeException accepts string keys (named arguments)
and prints them as-is in the message, keeping the #N form for int keys.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
…ure()

CacheTest mixed expected-first and actual-first Assert::same() calls;
the facade's signature is same($actual, $expected), so the inverted
calls produced swapped "expected X, got Y" messages on failure. All
calls now pass the actual value first.

CoroutineScopeInterceptor spelled out Failed/Error where an isFailure()
call away the same interceptor already uses the enum's own check —
one place to update if a failure-like status is ever added.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@roxblnfk
roxblnfk force-pushed the feat/fiber-coroutines branch from ffd69b2 to 5b38214 Compare August 8, 2026 11:25
…apping tests

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants