Skip to content

fix(runtime): update shell session wire protocol - #642

Open
priyasingh2111 wants to merge 1 commit into
mainfrom
fix/shell-fixes
Open

fix(runtime): update shell session wire protocol#642
priyasingh2111 wants to merge 1 commit into
mainfrom
fix/shell-fixes

Conversation

@priyasingh2111

Copy link
Copy Markdown

Issue #, if available:

Description of changes:
Updates the shell session wire protocol to simplify the connection handshake and remove deprecated features:

  • Remove metadata frame handshake: Connection is ready immediately after WebSocket 101 upgrade — shellId is now read from the response header instead of waiting for a STATUS frame. Eliminates the blocking `_read_metadata_frame()` step and the `_pending_frames` queue.
  • Remove `encode_close()` frame: `close()` no longer sends a CLOSE frame — the shell detaches and stays alive for the reconnect window.
  • Simplify `on_reconnect` callback: No longer passes a `reconnected: bool` argument — callback takes no parameters.
  • Remove `reconnected` and `bytes_dropped` attributes: No longer needed without the metadata frame flow.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@priyasingh2111
priyasingh2111 requested review from a team and padmak30 August 11, 2026 21:14
@github-actions github-actions Bot added the size/l PR size: L label Aug 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Breaking Change Warning

Found 3 potential breaking change(s) in this PR:

�[1msrc/bedrock_agentcore/runtime/shell/protocol.py�[0m:0: ShellFramer.encode_close: �[33mPublic object was removed�[39m
�[1msrc/bedrock_agentcore/runtime/shell/session.py�[0m:0: ShellSession.reconnected: �[33mPublic object was removed�[39m
�[1msrc/bedrock_agentcore/runtime/shell/session.py�[0m:0: ShellSession.bytes_dropped: �[33mPublic object was removed�[39m


Note: This is an automated static analysis check. Some flagged changes may be intentional.
Please confirm each item is expected and, if so, add a migration note to CHANGELOG.md.

@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Aug 11, 2026
reconnect_window: Optional[float] = _DEFAULT_RECONNECT_WINDOW
outer_loop_delay: float = _DEFAULT_OUTER_LOOP_DELAY
on_reconnect: Optional[Callable[[bool], Union[Awaitable[None], None]]] = field(default=None, repr=False)
on_reconnect: Optional[Callable[[], Union[Awaitable[None], None]]] = field(default=None, repr=False)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we update the open_shell() docstring in src/bedrock_agentcore/runtime/agent_core_runtime_client.py? It still shows on_reconnect(reconnected: bool) and says that exiting the context sends a CLOSE frame. The callback now takes no arguments, and exiting the context now detaches the shell.

I can also make this change after this is merged, but will reapprove if you update. Thanks.

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

Labels

size/l PR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants