Conversation
androidtv only clears `available` on an explicit close, so once the TV reset the ADB socket every tv tool call failed with ConnectionResetError until the server restarted. Probe the link with a no-op shell command before handing out the client, and close and reconnect when it fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
marvin-context-protocol
Bot
added
bug
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a75c3f6f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self._client.available and not await self._responds(self._client): | ||
| await self._client.adb_close() |
There was a problem hiding this comment.
When two tool calls run concurrently after the same socket reset, both can enter this branch after failed probes; one call may finish reconnecting and return the shared client before the other executes adb_close(), allowing the second call to close the newly restored connection while the first tool is using it. Protect the probe/close/connect sequence with a per-connection lock so only one caller performs recovery and subsequent callers observe the restored client.
AGENTS.md reference: AGENTS.md:L180-L182
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
The smart-home example's Fire TV tools failed permanently once the TV reset the ADB socket: every call raised
ConnectionResetErroruntil the server was restarted. The connection is now probed before each use and reopened when the probe fails.Cause
androidtvonly setsavailableto false on an explicit close. After the TV resets the socket the client still reportsavailable, soFireTVConnection.get()skipped its reconnect branch and handed out a dead connection.Seen on a running instance:
tv_read_statusreturned[Errno 104] Connection reset by peeron repeated calls while a fresh ADB connection to the same TV worked. What made the TV reset the socket is not established; a single sleep/wake cycle did not reproduce it.Change
FireTVConnection.get()sends a no-op shell command (true) when the client claims to be available, and closes the client if that raises, which lets the existing reconnect branch run.ConnectionResetErrorwhile still reportingavailableis reconnected, and the tool call succeeds.🤖 Generated with Claude Code