-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat[#1540]: Base changes for cli list sessions #1586
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
Conversation
| @@ -0,0 +1,33 @@ | |||
| use anyhow::Result; | |||
|
|
|||
| pub fn handle_session(verbose: bool, format: String) -> Result<()> { | |||
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.
rename to handle_session_list
| } else { | ||
| println!(" {}", id); | ||
| } | ||
| } |
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.
currently the output looks like this:
❯ goose session list
Available sessions:
20250307_121402
20250307_122909
20250307_125332
20250307_125441
20250307_112710
20250307_121343
20250310_112933
20250307_112732
20250307_124641
i think it'd be nicer to do sth similar to what we have in goose-server routes:
goose/crates/goose-server/src/routes/session.rs
Lines 47 to 79 in fa9bb9e
| let sessions = match session::list_sessions() { | |
| Ok(sessions) => sessions, | |
| Err(e) => { | |
| tracing::error!("Failed to list sessions: {:?}", e); | |
| return Err(StatusCode::INTERNAL_SERVER_ERROR); | |
| } | |
| }; | |
| let session_infos = sessions | |
| .into_iter() | |
| .map(|(id, path)| { | |
| // Get last modified time as string | |
| let modified = path | |
| .metadata() | |
| .and_then(|m| m.modified()) | |
| .map(|time| { | |
| chrono::DateTime::<chrono::Utc>::from(time) | |
| .format("%Y-%m-%d %H:%M:%S UTC") | |
| .to_string() | |
| }) | |
| .unwrap_or_else(|_| "Unknown".to_string()); | |
| // Get session description | |
| let metadata = session::read_metadata(&path).expect("Failed to read session metadata"); | |
| SessionInfo { | |
| id, | |
| path: path.to_string_lossy().to_string(), | |
| modified, | |
| metadata, | |
| } | |
| }) | |
| .collect(); |
then we can print sth like this:
[session_name] - [description] - [modified date]
salman1993
left a comment
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.
thanks for contributing. requested few changes on how sessions get listed
Thanks for the review! I tried to keep it simple and figured there would be some changes that needed to be made. I'll get working on the changes ASAP |
|
I made the requested changes by @salman1993 as well as some code maintainability changes. I held off on the --quiet flag as I want this initial scope to get into main and then we can start adding flags as needed |
salman1993
left a comment
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.
this is great!
* upstream/main: feat(google_drive): move credentials into keychain, add optional fallback (block#1603) feat: add session list command in cli (block#1586) feat: google sheets support (in google drive builtin MCP server) (block#1601) fix: deep link opening when window is closed (block#1633) docs: edits to docker guide (block#1639) feat: ollama tool shim (block#1448) feat: add write approve mode (block#1628) ui: auto update card upon config (block#1610) fix: fix tool output expansion checks (block#1634) fix: remove conditional that breaks output display for tool calls (block#1631) docs: Persistent Command History (block#1627) change to make build work on windows, macos, linux (block#1618) chore(release): release version 1.0.13 (block#1623) fix: handle mac screenshots with the image tool (block#1622) feat: write eval results to eval dir (block#1620) [fix] fix model config logging to remove api key (block#1619) fix: ensure repeating benches return to initial run-dir (block#1617)
I just reused what the ui was using to get the session list and then added a subcommand for "goose session". It checks to see if the subcommand is list and if not runs goose session as normal. I also added an example for arguments such as -v.