-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: remove unsafe pop of messages #47
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - I assume the "we have removed..." message still holds as exchange does it now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
src/goose/cli/session.py
Outdated
@@ -156,10 +156,10 @@ def run(self) -> None: | |||
self.reply() # Process the user message. | |||
except KeyboardInterrupt: | |||
self.interrupt_reply() | |||
except Exception: | |||
except FailedToGenerateMessageError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a related message in the other PR, I think a more generic exception catch does make sense here
square/exchange#30 (comment)
print(traceback.format_exc()) | ||
if self.exchange.messages: | ||
self.exchange.messages.pop() | ||
print( | ||
"\n[red]The error above was an exception we were not able to handle.\n\n[/]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe update this print statement to say a bit more clear about clearing the state to the just before the previous user prompt?
* main: fix: typo in exchange method `rewind` (#54) fix: remove unsafe pop of messages (#47) chore: Update LICENSE (#53) chore(docs): update is_dangerous_command method description (#48) refactor: improve safety rails speed and prompt (#45) feat: make goosehints jinja templated (#43) ci: enforce PR title follows conventional commit (#14) feat: show available toolkits (#37) adding in ability to provide per repo hints (#32) Apply ruff and add to CI (#40) added some regex based checks for dangerous commands (#38) chore: Update publish github workflow to check package versions before publishing (#19) chore: upgrade ai-exchange dependency (#36) fix: resuming sessions (#35) feat: upgrade `ai-exchange` to version `0.8.3` and fix tests (#34) fix: export metadata.plugins export should have valid module (#30) fix (#24) link to vs code extension (#20) Enable cli options for plugin (#22) Modified the readme to be more friendly to new users (#16)
* origin/main: chore: release 0.9.0 (block#58) fix: goose should track files it reads and not overwrite changes (block#46) docs: Small dev notes for using exchange from source (block#50) fix: typo in exchange method `rewind` (block#54) fix: remove unsafe pop of messages (block#47) chore: Update LICENSE (block#53) chore(docs): update is_dangerous_command method description (block#48) refactor: improve safety rails speed and prompt (block#45) feat: make goosehints jinja templated (block#43) ci: enforce PR title follows conventional commit (block#14) feat: show available toolkits (block#37) adding in ability to provide per repo hints (block#32) Apply ruff and add to CI (block#40) added some regex based checks for dangerous commands (block#38) chore: Update publish github workflow to check package versions before publishing (block#19) # Conflicts: # src/goose/toolkit/developer.py # src/goose/utils/check_shell_command.py # tests/utils/test_check_shell_command.py
Rewind to right before the last user message, and prompt them again