Skip to content
This repository was archived by the owner on Sep 28, 2025. It is now read-only.

implemented escape shell feature #30

Open
yyovil wants to merge 3 commits into
mainfrom
yyovil/fix-23
Open

implemented escape shell feature #30
yyovil wants to merge 3 commits into
mainfrom
yyovil/fix-23

Conversation

@yyovil
Copy link
Copy Markdown
Owner

@yyovil yyovil commented Aug 31, 2025

Currently i've rendered the escape shell message as a tool call which also gets added to the chat session so that the agent has it in its context. refactored the func (term *Terminal) Run(ctx context.Context, call ToolCall) (ToolResponse, error) method and broke it down into a reusable method func (term *Terminal) ExecuteCmd(ctx context.Context, cmd []string) (string, error). reused it to escape shell.

Closes #23

@yyovil yyovil self-assigned this Aug 31, 2025
@yyovil yyovil added the enhancement New feature or request label Aug 31, 2025
wasn't supposed to be included in the PR in the first place.
Copy link
Copy Markdown
Owner Author

@yyovil yyovil left a comment

Choose a reason for hiding this comment

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

Implement suggested changes and your work will be accepted without any further review.

session session.Session
textarea textarea.Model
attachments []message.Attachment
EscapeShellMode bool
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

adhere to naming convention if export isn't necessary which i guess is the case in here.

})

// Run the tool (reuse ExecuteCmd for execution) without shell wrapping
tool := tools.NewDockerCli().(*tools.Terminal)
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

We should use singleton instead of initializing new Docker CLI every time escapeShell is invoked

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

We can reuse the tool used by the agents here

}},
})

// If we had to create a new session locally, notify the rest of the app
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

We should let user decide whether or not the escape shell commands ran by the user should be included in the llm context

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Initially, it should be done via configuration file swarm.json and eventually it will be done using TUI as well.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Do not render escape shell command message as a tool call, render it as a system message

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shell escape feature in tandem.

1 participant