Skip to content

[Repo Assist] fix: correct Async.bind signature and implementation#394

Draft
github-actions[bot] wants to merge 2 commits intomainfrom
repo-assist/fix-async-bind-20260419-50fde4ecdc2613e3
Draft

[Repo Assist] fix: correct Async.bind signature and implementation#394
github-actions[bot] wants to merge 2 commits intomainfrom
repo-assist/fix-async-bind-20260419-50fde4ecdc2613e3

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This is an automated pull request from Repo Assist, an AI assistant.

Summary

Async.bind in Utils.fs had the same bug that Task.bind was fixed for in commit 8486e1b: the binder function received the entire Async<'T> wrapper rather than the awaited inner value 'T.

Root Cause

The previous implementation:

let inline bind binder (async: Async<'T>) : Async<'U> =
    ExtraTopLevelOperators.async { return! binder async }

And its corresponding signature:

val inline bind: binder: (Async<'T> -> Async<'U>) -> async: Async<'T> -> Async<'U>

The binder received Async<'T> — effectively making Async.bind equivalent to plain function application (fun binder async -> binder async), not a monadic bind. This was inconsistent with Task.bind which correctly has binder: ('T -> #Task<'U>).

Fix

Both the implementation and the signature have been corrected to standard monadic bind:

let inline bind binder (async: Async<'T>) : Async<'U> =
    ExtraTopLevelOperators.async {
        let! result = async
        return! binder result
    }
val inline bind: binder: ('T -> Async<'U>) -> async: Async<'T> -> Async<'U>

Breaking Change

This changes the public API signature. The previous signature was incorrect and the function was likely never useful as published (any code using it would have needed a fun (asyncT: Async<'T>) -> ... binder rather than the natural fun (t: 'T) -> ... form). A suppression has been added to CompatibilitySuppressions.xml.

Added Tests

Utils.Tests.fs is a new test file covering Async.bind, Task.bind, Async.map, and Task.map — none of which had dedicated tests before.

Test Status

  • Build: ✅ dotnet build -c Release — succeeded (0 warnings, 0 errors)
  • Tests: ✅ dotnet test -c Release — 5190 passed, 2 skipped (pre-existing infrastructure skips)
  • Format: ✅ dotnet fantomas . --check — passed

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

The previous signature was (Async<'T> -> Async<'U>) -> Async<'T> -> Async<'U>,
which passed the entire Async<'T> computation to the binder without first
awaiting it. This made the function essentially equivalent to plain function
application, not a monadic bind.

The correct signature is ('T -> Async<'U>) -> Async<'T> -> Async<'U>, matching
Task.bind (which had the same bug fixed in commit 8486e1b) and standard
monadic bind semantics.

Updated CompatibilitySuppressions.xml to suppress the CP0002 baseline-breaking
change diagnostic, since this is an intentional fix of an incorrect public API.

Added Utils.Tests.fs with tests covering the corrected behavior of
Async.bind, Task.bind, Async.map, and Task.map.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants