Skip to content

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

Merged
dsyme merged 3 commits into
mainfrom
repo-assist/fix-async-bind-20260419-50fde4ecdc2613e3
Apr 20, 2026
Merged

[Repo Assist] fix: correct Async.bind signature and implementation#394
dsyme merged 3 commits into
mainfrom
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 inlinebind binder (async:Async<'T>):Async<'U>=
ExtraTopLevelOperators.async{return! binder async }

And its corresponding signature:

val inlinebind: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 inlinebind binder (async:Async<'T>):Async<'U>=
ExtraTopLevelOperators.async{let!result= async
return! binder result
}
val inlinebind: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>
@dsyme
dsyme marked this pull request as ready for review April 20, 2026 02:30
@dsyme
dsyme merged commit 3b27740 into mainApr 20, 2026
5 checks passed
@dsyme
dsyme deleted the repo-assist/fix-async-bind-20260419-50fde4ecdc2613e3 branch April 20, 2026 02:35
@bartelink

bartelink commented Apr 24, 2026

Copy link
Copy Markdown
Member

@dsyme Any reason the Async.bind impl can't just call ExtraTopLevelOperators.async.Bind ?
(Seems to work and pass the tests locally)
(Asking as compiling a fslang-suggestion re transplanting some of these helpers)
I can also see the case for having it implemented directly, so let me know if you want/see the point in doing a PR

Sign up for freeto 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.

2 participants

@bartelink@dsyme