canary: migrate the banana bot to mcp 2.x; e2e: keep failure evidence - #362
Merged
Merged
Conversation
The cop canary installs an unpinned 'mcp'; the 2.0 release renamed streamablehttp_client to streamable_http_client and moved headers into an injected http_client, so the pod crash-looped on ImportError and the bananas deploy failed. Migrate the snippet the same way as sam-mcp-python's client and pin the canary install to the 2.x major.
The harness already dumps container logs to tests/e2e/logs/ for tests that did not complete, but CI only uploaded ./_artifacts, which bats never populates - so the a2a label-gate flake left nothing to diagnose. Upload the dump directory, and print node-1's [A2A] gate verdicts inline when the labelled send fails, since that gate fail-closes transient handshake errors into the same 403 as a policy denial.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request pins the mcp dependency version in the Kubernetes template, fixes an import name and client initialization in banana_bot_playground.py (introducing an explicit httpx.AsyncClient), and adds diagnostic logging to the end-to-end mesh tests on failure. Feedback was provided to wrap the connection logic in a try...except block to prevent httpx.AsyncClient resource leaks if the connection process fails.
Review catch: connect() failures inside __aenter__'s 12-attempt retry loop orphaned that attempt's httpx.AsyncClient and half-entered streams, since the next attempt overwrites the attributes close() reaches. GC cannot help - aclose() is async and a finalizer cannot await it, and the pool's reference cycles defer even fd closure to an arbitrary cyclic-GC pass. Same try/except-close pattern as sam_mcp's client.
…shakes The uploaded verdict names the flake: 'labels unverifiable: failed to open auth stream: ... context deadline exceeded' - the biscuit-fetch stream open timing out under -j10 runner load, not a policy denial. Verdicts are stable, the transport is not, and the gate caches only positive verdicts, so a bounded retry re-fetches honestly. Each failed attempt still prints node-1's [A2A] verdicts; the mismatched-label 403 assertion stays unretried.
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
1 participant
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.
Two independent CI/deploy fixes:
Deploy failure (run 34087805318): the cop canary installs an unpinned
mcpand the 2.0 release renamedstreamablehttp_client->streamable_http_client(headers now ride an injectedhttp_client), so the pod crash-looped on ImportError and the bananas rollout timed out. The banana bot snippet was the only place left on the 1.x API after the repo-wide migration. Migrated it the same way assam-mcp-python's client and pinned the canary install tomcp>=2,<3.e2e flake diagnosability (run 34085662241): the a2a label-gate test flaked with a bare 403 and no node logs. The harness dumps container logs to
tests/e2e/logs/for failed tests, but CI uploaded only./_artifacts, which bats never populates. Now the dump directory is uploaded, and the test prints node-1's[A2A]gate verdicts inline on failure — the gate fail-closes transient biscuit-fetch errors into the same 403 as a policy denial, and next time we'll see which one it was. Deliberately no retry until the root cause is known.