Skip to content

test(ag2-sparrow): wait for the orphaned provider child's record, not its file - #6051

Open
sonichi wants to merge 2 commits into
mainfrom
fix/cloud-session-orphan-child-flake
Open

sonichi wants to merge 2 commits into
mainfrom
fix/cloud-session-orphan-child-flake

Conversation

@sonichi

@sonichi sonichi commented Oct 30, 2026

Copy link
Copy Markdown
Owner

What changed, and why

packages/ag2-sparrow/tests/test_cloud_session_contract.py test_a_provider_child_outliving_the_adapter_produces_at_most_one_turn (run through tests/ag2-sparrow-cloud-session-contract.test.py) failed with AssertionError: '' != 'call\n' on #6050's run 41089392669. That PR does not touch this code.

Root cause. The test waited for calls.exists(). The fake provider writes open(calls, 'a').write('call\n'), and open() creates the file before the write lands. A poll that fell between the two read an empty file. Fix. Wait for the record itself: the file exists and is non-empty. Append mode stays, so a resend still shows as a second call line and the "never resends" check keeps its teeth. Test-only, about 2 lines; no timeout raised, no retry.

Before / after evidence

To widen the window deterministically, the fake provider sleeps 0.5 s between open and write:

origin/main test + widened window:  AssertionError: '' != 'call\n'   FAILED
this branch      + widened window:  OK

That is the exact CI error.

Frequency

1 confirmed hit (41089392669). I also checked the three main runs named alongside it (41074235384, 41073413124, 41001271257). This file passed in all three; those runs failed on pool-restart and ack-claim-lock-cleanup. In CI runs since 14:50Z, the other repeat failures are hatch-adapter (2, fixed by #6050), runtime-api-e2e (2 hits overall; not addressed here), roster-preflight (1) and lane-dedupe (1, fixed by #6048).

Tests run (macOS, nice, 5x)

python3 tests/ag2-sparrow-cloud-session-contract.test.py: 5/5 OK, 10 tests, about 6 s each. review-checks.sh: PASS; ruff: clean.

Documentation

N/A. Test only.

Related: #6048, #6049, #6050.

Review: @qingyun-mini.agent:s.space

🤖 Generated with Claude Code

… its file

test_a_provider_child_outliving_the_adapter_produces_at_most_one_turn polled
for calls.exists(), but the fake provider's open(..., 'a') creates the file
before its write lands, so a poll in between read '' and failed with
"'' != 'call\\n'". The wait now ends on the written record. Append mode is
kept, so a resend still shows as a second line.

Stand: Blue (@chi-chi-air-blue.agent:s.space)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonichi
sonichi requested a review from qingyun-wu as a code owner October 30, 2026 16:50
Stand: Blue (@chi-chi-air-blue.agent:s.space)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 94%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • packages/ag2-sparrow/tests/test_cloud_session_contract.py (100%)

Summary

  • Total: 1 line
  • Missing: 0 lines
  • Coverage: 100%

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified at 2bdd836.

  • Root cause holds. The fake provider writes the record with open(calls, 'a').write('call\n'), so the file exists before the write is flushed, and the old calls.exists() poll could read ''. That is the reported CI error.
  • The fix waits for a complete line (endswith('\n')). It keeps append mode and the same deadline, so the later "never resends" assertion still catches a second call line. The started.exists() wait above it needs existence only, so it has no such race.
  • Test-only, two lines. The comment fits the two-line rule.
  • CI is green at this head: all required checks pass (Windows/publish/release skipped).

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve at 2bdd836d. Ready to merge.

Evidence for the bar

  • Expert approval: @qingyun-wu APPROVED at this head with a substantive review. It traces the root cause to the fake provider's open(calls, 'a').write('call\n'), confirms append mode keeps the later "never resends" check able to catch a second call line, and confirms the started.exists() wait above it has no such race.
  • Verification: the author widened the window deterministically (a 0.5 s sleep between open and write). origin/main's test fails with the exact CI error ('' != 'call\n'); this branch passes.

Triage walk

  • Close: no. It is test-only, but it fixes a flaky wait in an existing test, not a new assertion.
  • Change-request conditions: none fire. The test drives the real adapter and provider child; it is not a source grep or an in-file copy. The diff is 2 lines and changes no timeout. No hardcoded paths. Not a live path.
  • §3.3 test standard: the new wait condition is deterministic (it waits for the record, not the file), and a failure still points at the invariant.

Gates at this head: 48 checks, none red. license/cla success (1 context). 0 unresolved threads. mergeStateStatus CLEAN.

— Sutando (susans-MacBook-Pro)

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants