feat: Add save-session metaquery and ask to save on exit#484
Conversation
| log.Info("Received input from channel", "userInput", userInput) | ||
| if userInput == io.EOF { | ||
| log.Info("Agent loop done, EOF received") | ||
| if _, ok := c.ChatMessageStore.(*sessions.InMemoryChatStore); ok { |
There was a problem hiding this comment.
There is another way to do this where UI (terminal UI in this case) can present this option of saving or not at their end and then can send "save-session" meta query to the agent.
That approach decouples the UI from the core agent layer where agent isn't aware about user's interaction etc. Another advantage is that it unifies the "saving session" flow where there is only one mechanism (issuing meta query) to save session.
WDYT ?
There was a problem hiding this comment.
I added handling for this on the the terminal side. Manually tested stdin path by ./kubectl-ai "query" and tty path by echo "query" | ./kubectl-ai
| } | ||
| foundSession, _ := manager.FindSessionByID(c.session.ID) | ||
| if foundSession != nil { | ||
| return "", fmt.Errorf("session with ID %s already exists", foundSession.ID) |
There was a problem hiding this comment.
if user is already in a session that being persisted and they say "save-session" just to ensure that session is being saved, will that result in error ? (probably, we should respond with a more friendly message saying.... session is already being saved or something....)
There was a problem hiding this comment.
I removed the error, now it just returns that the session was saved. I don't know how important it is to distinguish to the user that the session is already being saved vs. newly being saved. If you think that's a big deal, I can change it to include an already being saved message.
| if foundSession != nil { | ||
| return "", fmt.Errorf("session with ID %s already exists", foundSession.ID) | ||
| } | ||
| metadata := sessions.Metadata{ |
There was a problem hiding this comment.
if the session was in-memory then this metadata should exists somewhere right ? because session was created sometime ago and we decided to save it just now. It is like migrating the session from in-memory to persistent-session so sort of attaching the persistence layer.
There was a problem hiding this comment.
I updated this to use the existing CreatedAt, though the rest of the metadata is fine as is (LastAccessed, ModelID, ProviderID).
noahlwest
force-pushed
the
save-session
branch
from
a2e6324 to
628c996
Compare
2 participants
Lets people start without a persisted session and switch at any time with save-session
Also, adds a check on ctrl+c exit whether user wants to save the current in-memory session.
demo (shows use of save-session, yes on exit save, and no on exit save): https://github.com/user-attachments/assets/dccb62e9-3f48-4b7d-b897-ac1ea6cb23ba