fix: replace asyncio.sleep FAF guards with deterministic awaits#919
Merged
ajbozarth merged 4 commits intoApr 27, 2026
Merged
Conversation
Bumps cpex to 0.1.0.dev12, which exposes PluginResult.background_tasks and wait_for_background_tasks(). Uses these to eliminate all racy asyncio.sleep() calls that were guarding FIRE_AND_FORGET plugin completion in tests. - Add drain_background_tasks() to mellea/plugins/manager.py: accumulates PluginResult objects with background tasks in invoke_hook(), drains via wait_for_background_tasks() in tests - test/plugins/test_execution_modes.py: capture invoke_hook result and call result.wait_for_background_tasks() directly - test/telemetry/test_metrics_backend.py: replace 7x asyncio.sleep(0.05) with drain_background_tasks() Closes generative-computing#691 Assisted-by: Claude Code Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
Add discard_background_tasks() to clear accumulated FIRE_AND_FORGET results without awaiting them, and call it in the enable_metrics fixture to discard stale results from previous test event loops before each test. Also clears _pending_background_results in shutdown_plugins() for completeness. Assisted-by: Claude Code Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
Contributor
Author
|
Follow-up to address a test failure in The Fix: added Also clears |
Contributor
Author
|
Claude decided to delete my worktree for this PR instead of #895 when I went to clean up and I didn't noticed it picked the wrong branch name |
Contributor
Author
|
cc @araujof as the original plugins/hook author |
planetf1
requested changes
Apr 27, 2026
Contributor
planetf1
left a comment
planetf1
left a comment
There was a problem hiding this comment.
memory leak to consider
Adds `_collect_background_results` boolean (default False) so `_pending_background_results` is never populated in production. Collection is enabled/disabled explicitly by test fixtures via the new `enable_background_collection()` / `disable_background_collection()` helpers, eliminating the unbounded growth on metrics-enabled servers. Also replaces "FAF" abbreviation in docstrings with "fire-and-forget" for clarity. Assisted-by: Claude Code Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
Contributor
Author
|
@planetf1 Your review should be addressed if you want to take another look |
Merged
via the queue into
generative-computing:main
with commit Apr 27, 2026
bd7fa38
7 checks passed
akihikokuroda
pushed a commit
to akihikokuroda/mellea
that referenced
this pull request
May 27, 2026
…rative-computing#919) * fix: replace asyncio.sleep FAF guards with deterministic awaits Bumps cpex to 0.1.0.dev12, which exposes PluginResult.background_tasks and wait_for_background_tasks(). Uses these to eliminate all racy asyncio.sleep() calls that were guarding FIRE_AND_FORGET plugin completion in tests. - Add drain_background_tasks() to mellea/plugins/manager.py: accumulates PluginResult objects with background tasks in invoke_hook(), drains via wait_for_background_tasks() in tests - test/plugins/test_execution_modes.py: capture invoke_hook result and call result.wait_for_background_tasks() directly - test/telemetry/test_metrics_backend.py: replace 7x asyncio.sleep(0.05) with drain_background_tasks() Closes generative-computing#691 Assisted-by: Claude Code Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com> * fix: isolate FAF plugin results per test to prevent cross-loop errors Add discard_background_tasks() to clear accumulated FIRE_AND_FORGET results without awaiting them, and call it in the enable_metrics fixture to discard stale results from previous test event loops before each test. Also clears _pending_background_results in shutdown_plugins() for completeness. Assisted-by: Claude Code Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com> * fix: gate background result collection behind opt-in flag Adds `_collect_background_results` boolean (default False) so `_pending_background_results` is never populated in production. Collection is enabled/disabled explicitly by test fixtures via the new `enable_background_collection()` / `disable_background_collection()` helpers, eliminating the unbounded growth on metrics-enabled servers. Also replaces "FAF" abbreviation in docstrings with "fire-and-forget" for clarity. Assisted-by: Claude Code Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com> --------- Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Misc PR
Type of PR
Description
Bumps
cpexto0.1.0.dev12, which addsPluginResult.background_tasksandwait_for_background_tasks()(upstream contextforge-org/contextforge-plugins-framework#33). Uses these to replace all racyasyncio.sleep()guards that were used to wait for FIRE_AND_FORGET plugin completion in tests.Changes:
mellea/plugins/manager.py: adddrain_background_tasks()— accumulatesPluginResultobjects with background tasks ininvoke_hook(), drains viawait_for_background_tasks()for tests where the hook fires inside a backend calltest/plugins/test_execution_modes.py: captureinvoke_hookresult directly and callresult.wait_for_background_tasks()(2 tests)test/telemetry/test_metrics_backend.py: replace 7×asyncio.sleep(0.05)withdrain_background_tasks()Testing
Attribution