Skip to content

fix(redis): propagate listener startup failures to senders - #3503

Open
xyi74976-del wants to merge 2 commits into
ag2ai:mainfrom
xyi74976-del:fix/redis-stream-startup-failure-oct26
Open

xyi74976-del wants to merge 2 commits into
ag2ai:mainfrom
xyi74976-del:fix/redis-stream-startup-failure-oct26

Conversation

@xyi74976-del

@xyi74976-del xyi74976-del commented Oct 25, 2026 •

Copy link
Copy Markdown

Why are these changes needed?

RedisStream.send() waits only for the listener's readiness event. When the initial Redis subscription fails, the listener exits before setting that event and waiting senders hang. Closing during startup has the same problem; the PubSub object also needs cleanup when subscription fails.

Wait for readiness or the captured listener task, propagating its startup error/cancellation. Each sender releases its own readiness waiter without cancelling the shared listener. Subscription startup is inside the cleanup scope, PubSub closes even if unsubscribe fails, and a later send can retry.

The branch now includes upstream main fa4e0fb via bc7d012. Only the original Redis source and test files differ from that base. The sync includes upstream #3510 and #3492, which address the five OpenAI usage assertion failures and A2UI validation timeout seen in the previous hosted run. No unrelated assertion rewrite, timeout relaxation or CI-policy change is authored here.

Related issue number

No matching issue found. Merged #3018 handles listener cancellation after subscription; this PR covers the remaining startup and cleanup paths.

Checks

  • I've included any doc changes needed for https://docs.ag2.ai/. No public API change.
  • I've added tests corresponding to the changes introduced in this PR.
  • I've made sure all auto checks have passed. New-head hosted CI is pending; local results below do not claim remote approval.

Current-head hosted workflows require maintainer approval: PR checks, Tests, and Git LFS Check. They have not executed on bc7d012; the passed local validation below is separate.

Validation of the synchronized source on macOS / Python 3.12.15:

  • Original Redis regression: 10 failures / 2 passes on unchanged source, 12 passes after repair, across JSON/pickle startup failure, concurrent senders, retry, cancellation and cleanup.
  • 352 focused tests pass, including Redis, the previously failing OpenAI/A2UI cases and upstream stream/observer/tool timeout suites.
  • Official just test-cov, with CI's test and optionals dependency groups: 7,855 passed, 6 skipped, 433 marker-deselected, 2 xfailed; coverage reporting also passed.
  • Full mypy: 255 source files pass. All pre-commit checks pass with CI's existing SKIP=mypy; mypy ran separately. No new waiver.
  • Independent AI integration review found no actionable issue and reran the Redis tests: 52 passed.

The five prior OpenAI failures also reproduced locally before synchronization. The A2UI timeout was observed in hosted Python 3.10 CI and did not reproduce locally. Redis tests use the repository's mocked boundary. Native Windows, other Python versions and external Redis/provider services were not run locally. Existing dependency warnings and optional/platform skips remain.

AI assistance

OpenAI Codex investigated the bug, wrote the implementation and tests, checked the actual diff, and ran the validation above. A separate Codex reviewer inspected the patch and reran the Redis tests. The contributor has confirmed reading and understanding revision bc7d012, reviewing its validation results, and taking responsibility for the change.

  • I understand the changes in this PR and can explain them in my own words.
  • I have verified that the PR description accurately reflects the actual diff. (Verified by Codex.)
  • If AI assistance was used, I reviewed, tested, and validated the generated code/text before submitting.

@github-actions github-actions Bot added the area:core Agent runtime core: ag2/*.py, ag2/events, ag2/response, ag2/streams label Oct 25, 2026
@CLAassistant

CLAassistant commented Oct 25, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@vvlrff vvlrff added the status:needs-cla Gate: CLA not signed; not reviewed until signed label Oct 26, 2026
@vvlrff

vvlrff commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Hi @xyi74976-del, thanks for the contribution!

Before we can review or merge this PR, you need to sign our Contributor License Agreement — the license/cla check on this PR has the signing link. This applies to draft PRs as well: we don't want you to invest work into a change we can't accept.

Once the CLA is signed, the check turns green, the status:needs-cla label is removed, and the PR proceeds through the normal flow.

⏳ If the CLA isn't signed within 3 days, this PR will be marked stale and closed 7 days after that. Signing later and reopening is always an option.

@vvlrff vvlrff removed the status:needs-cla Gate: CLA not signed; not reviewed until signed label Oct 27, 2026

Copy link
Copy Markdown
Author

@vvlrff The CLA checks for #3503 and #3504 now pass, and the contributor's human-review acknowledgements are complete. The current heads are bc7d012 and 37dd672, both synchronized with main fa4e0fb. The previous #3503 run's OpenAI assertion failures and A2UI validation timeout are addressed by upstream #3510/#3492. Current local just test-cov results are 7,855 and 7,852 passed respectively; full mypy and pre-commit checks also pass.

The new-head workflows remain action_required: https://github.com/ag2ai/ag2/actions/runs/40640956087 and https://github.com/ag2ai/ag2/actions/runs/40642575752 . Could a maintainer approve the pending runs and review the fixes when convenient? No passed hosted matrix is claimed. AI-assisted follow-up, verified against the current heads.

This branch has not been deployed

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

Labels

area:core Agent runtime core: ag2/*.py, ag2/events, ag2/response, ag2/streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants