Skip to content

Fix async task review regressions #126

Description

@D3f0

Problem

Post-merge review of PR #121 reproduced three correctness regressions in async task support:

  1. hide=True leaks stdout/stderr from the async runner.
  2. Synchronous tasks in an invocation containing an async task receive a coroutine from ctx.run() and silently skip their command.
  3. Grouped pre/post task entries can crash async-task detection before execution.

The review also found an under-annotated public ToolkitContext.run surface and documented async-path limitations.

Acceptance criteria

  • Async hide=True suppresses both terminal streams while retaining captured Result output.
  • Only coroutine task bodies execute inside async_task_context; synchronous graph members retain blocking ctx.run() semantics.
  • Async-task detection recursively handles grouped pre/post entries; execution does not treat grouping containers as tasks.
  • ToolkitContext.run exposes the synchronous-or-awaitable Result return contract to type checkers.
  • Async documentation removes resolved limitations and retains the current buffered-output, non-interactive/watchers, sudo, and sync-helper caveats.
  • Focused async/grouped-task tests cover each regression.

Reference: docs/async-review-findings.md.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions