Skip to content

feat(shell): support detaching from remote shell - #2609

Merged
notgitika merged 3 commits into
refactorfrom
detach-shell-command
Oct 19, 2026
Merged

notgitika merged 3 commits into
refactorfrom
detach-shell-command

Conversation

@nborges-aws

@nborges-aws nborges-aws commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Description

Add Ctrl+] as keybind to detach from a runtime or harness shell, keeping the remote shell alive. The local terminal is restored and a reconnect command with session and shell id is printed, allowing for reconnection. Normal shell exit behavior is unaffected by this change.

This change is dependent on: aws/bedrock-agentcore-sdk-typescript#249, which enables the remote shell to stay open when detach is sent to the SDK. For detach to be fully functional in the CLI, this change will need to be released in TS SDK and the pinned SDK version will need to be updated in the CLI.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe):

Testing

How have you tested the change?

  • I ran bun test (x pass, 0 fail)
  • I ran the relevant end-to-end tests with bun run test:e2e, or explained why they are not applicable
  • I ran bun run typecheck
  • I ran bun run lint:check
  • I ran bun run format:check
  • I ran bun run build
  • If I modified src/assets/, I updated affected snapshots with bun test <test-file> --update-snapshots and committed them

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

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


Stack created with GitHub Stacks CLI • Give Feedback 💬

@nborges-aws
nborges-aws added this pull request to stack #2610 October 16, 2026 15:09
@github-actions github-actions Bot added the size/m PR size: M label Oct 16, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Oct 16, 2026

@agentcore-devx-automation agentcore-devx-automation Bot left a comment

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.

AgentCore Harness Review

Verdict: Looks good

Nice, focused change. The detach handling in InteractiveTerminal.run is coherent:

  • onData sets didDetach before enqueueing to ensure further input (and post-escape bytes in the same chunk) are intentionally dropped.
  • Pre-escape bytes are still forwarded; the detach callback and resolveDetach() are serialized through the same pending queue so ordering is correct.
  • shell.ts correctly skips the finally-block session.close() when onDetach already closed the peer, avoiding a double-close, and the test asserts closes === 1.
  • Removing reconnected from RuntimeShellSdkSession is safe — it was never read; reconnection status is already delivered via metadata/onReconnect in harnessShell.ts.

A couple of minor observations that aren't blockers:

  • When onDetach is set, Ctrl+] can no longer be forwarded to the remote shell (e.g., for tmux prefix). This is intentional given the UX, but worth keeping in mind if someone later asks for an escape to send a literal 0x1d.
  • If onDetach itself were to throw, resolveDetach isn't called and the operation surfaces the queued failure instead of returning "detached". That's reasonable, though in practice session.close() is unlikely to reject.

No telemetry gap introduced — the shell handler doesn't currently emit telemetry, so this change isn't a regression there.

@agentcore-devx-automation agentcore-devx-automation Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Oct 16, 2026
@codecov-commenter

codecov-commenter commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.61%. Comparing base (d96d9d3) to head (bdcc519).

Additional details and impacted files
@@            Coverage Diff            @@
##           refactor    #2609   +/-   ##
=========================================
  Coverage     96.61%   96.61%           
=========================================
  Files           702      702           
  Lines         58318    58344   +26     
=========================================
+ Hits          56343    56370   +27     
+ Misses         1975     1974    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nborges-aws
nborges-aws marked this pull request as draft October 16, 2026 15:20
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Oct 16, 2026

@notgitika notgitika left a comment

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.

One thing to address with the bedrock-agentcore version bump: the SDK change that makes close() detach also changes reconnectConfig.onReconnect from (reconnected: boolean) => ... to () => ....

The CLI still passes the boolean callback through in src/core/runtimeShell.ts and src/core/harnessShell.ts, so using the new SDK types currently fails typecheck. Could you update RuntimeShellRequest/ShellOperation to use the no-argument callback and adjust the reconnect message and tests as part of the version bump?

@nborges-aws
nborges-aws force-pushed the detach-shell-command branch 2 times, most recently from 9acc92b to 33a5f52 Compare October 19, 2026 14:01
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Oct 19, 2026
@notgitika
notgitika force-pushed the detach-shell-command branch from 33a5f52 to 0350a63 Compare October 19, 2026 15:13
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Oct 19, 2026
Base automatically changed from shell-resume-command to refactor October 19, 2026 15:25
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Oct 19, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Oct 19, 2026
@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 Oct 19, 2026
@notgitika
notgitika force-pushed the detach-shell-command branch from b73b855 to a91e23a Compare October 19, 2026 16:41
@github-actions github-actions Bot removed the size/m PR size: M label Oct 19, 2026
@github-actions github-actions Bot added the size/m PR size: M label Oct 19, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Oct 19, 2026
@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 Oct 19, 2026
@nborges-aws
nborges-aws marked this pull request as ready for review October 19, 2026 18:17
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Oct 19, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Oct 19, 2026
@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 Oct 19, 2026

@notgitika notgitika left a comment

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.

Minor, non-blocking: a resize event during the narrow detach window can cause the CLI to report an error even though the remote shell was detached successfully. This can be handled as a follow-up.

Otherwise LGTM!

@notgitika
notgitika merged commit 2561439 into refactor Oct 19, 2026
18 checks passed
@notgitika
notgitika deleted the detach-shell-command branch October 19, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants