-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fix working directory when session has no messages #3513
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 working directory when session has no messages #3513
Conversation
2b7e5a3 to
c16fe19
Compare
Signed-off-by: Soroosh <[email protected]>
c16fe19 to
ab49a8d
Compare
|
@michaelneale In the description I outlined how the issue can be reproduced. |
|
Thank you for outlining the issue and how to reproduce it! For reference, this is happening for users on Mac and Linux #2790 |
| std::env::current_dir() | ||
| .or_else(|_| Ok::<PathBuf, io::Error>(get_home_dir())) | ||
| .expect("could not determine the current working directory") | ||
| } |
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.
excellent. I wonder if we should avoid panicking here though - either return a default "", or bubble up?
fn get_current_working_dir() -> Result<PathBuf, io::Error> {
std::env::current_dir().or_else(|_| Ok(get_home_dir()))
}
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.
yeah - in other cases if we can't find a cwd then default to home (I think that may only show up in rare testing scenarios though). Things will work if you run from home (as it is usually smart enough to then know what directory to really work in from context later on) - so seems reasonable to default instead of panic?
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.
Here we default to home dir and then panic. Isn't this the same behaviour as you outlined?
Note that get_home_dir itself also panic if it can't find the home dir. So I think the only question is whether want want to show a specific error here that cwd couldn't be determined, or home directory couldn't be determined.
* main: (25 commits) fix: add maintainer, homepage and categories to DEB/RPM package config (#3096) blog: agent to agent convo (#3677) Possible to disable random thinking messages (#3304) Two VS code tutorials (#3603) small blog fixes (#3549) docs: fix installation command for YouTube Transcript MCP in servers.json (#3595) Docs for using Docker Model Runner as a local LLM provider. (#3509) Docs: VS Code Extension move to tutorials (#3601) Fix working directory when session has no messages (#3513) goose docs MCP server (#3665) Remove confusing status output when testing sharing url connection and it shows 404 (#3659) chore: use typed notifications from rmcp (#3653) feat: convert GetPromptResult from mcp_core to rmcp version (#3650) feat: Replace usage of mcp_core Tools/ToolAnnotations in openapi schema (#3649) fix: ensure execution task result is shown (#3629) docs: Quick spotlight fix (#3633) alexhancock/rmcp-tools-annotations (#3617) fix: clean up subagent (#3565) Adds the `WaitingForUserInput` state (#3620) docs: update extensions library (#3612) ...
* main: (27 commits) Add inline python extension (#3107) fix: add maintainer, homepage and categories to DEB/RPM package config (#3096) blog: agent to agent convo (#3677) Possible to disable random thinking messages (#3304) Two VS code tutorials (#3603) small blog fixes (#3549) docs: fix installation command for YouTube Transcript MCP in servers.json (#3595) Docs for using Docker Model Runner as a local LLM provider. (#3509) Docs: VS Code Extension move to tutorials (#3601) Fix working directory when session has no messages (#3513) goose docs MCP server (#3665) Remove confusing status output when testing sharing url connection and it shows 404 (#3659) chore: use typed notifications from rmcp (#3653) feat: convert GetPromptResult from mcp_core to rmcp version (#3650) feat: Replace usage of mcp_core Tools/ToolAnnotations in openapi schema (#3649) fix: ensure execution task result is shown (#3629) docs: Quick spotlight fix (#3633) alexhancock/rmcp-tools-annotations (#3617) fix: clean up subagent (#3565) Adds the `WaitingForUserInput` state (#3620) ...
* main: (69 commits) Add inline python extension (#3107) fix: add maintainer, homepage and categories to DEB/RPM package config (#3096) blog: agent to agent convo (#3677) Possible to disable random thinking messages (#3304) Two VS code tutorials (#3603) small blog fixes (#3549) docs: fix installation command for YouTube Transcript MCP in servers.json (#3595) Docs for using Docker Model Runner as a local LLM provider. (#3509) Docs: VS Code Extension move to tutorials (#3601) Fix working directory when session has no messages (#3513) goose docs MCP server (#3665) Remove confusing status output when testing sharing url connection and it shows 404 (#3659) chore: use typed notifications from rmcp (#3653) feat: convert GetPromptResult from mcp_core to rmcp version (#3650) feat: Replace usage of mcp_core Tools/ToolAnnotations in openapi schema (#3649) fix: ensure execution task result is shown (#3629) docs: Quick spotlight fix (#3633) alexhancock/rmcp-tools-annotations (#3617) fix: clean up subagent (#3565) Adds the `WaitingForUserInput` state (#3620) ...
…cn/compact2-task-tracking * 'dkatz/goose-compact2' of github.com:block/goose: (22 commits) rm stray files unused fmt fix threshold autocompact splice last message fmt Fix conversations before they hit the LLM (#3660) cli: add detailed instruction for WSL users (#3496) feat: recipe runs will now prompt for missing extension secrets (#3668) fix: pricing integration tests -> trying more runs for cache and retries (#3546) Add inline python extension (#3107) fix: add maintainer, homepage and categories to DEB/RPM package config (#3096) blog: agent to agent convo (#3677) Possible to disable random thinking messages (#3304) Two VS code tutorials (#3603) small blog fixes (#3549) docs: fix installation command for YouTube Transcript MCP in servers.json (#3595) Docs for using Docker Model Runner as a local LLM provider. (#3509) Docs: VS Code Extension move to tutorials (#3601) Fix working directory when session has no messages (#3513) ...
* dkatz/goose-compact2: (22 commits) rm stray files unused fmt fix threshold autocompact splice last message fmt Fix conversations before they hit the LLM (#3660) cli: add detailed instruction for WSL users (#3496) feat: recipe runs will now prompt for missing extension secrets (#3668) fix: pricing integration tests -> trying more runs for cache and retries (#3546) Add inline python extension (#3107) fix: add maintainer, homepage and categories to DEB/RPM package config (#3096) blog: agent to agent convo (#3677) Possible to disable random thinking messages (#3304) Two VS code tutorials (#3603) small blog fixes (#3549) docs: fix installation command for YouTube Transcript MCP in servers.json (#3595) Docs for using Docker Model Runner as a local LLM provider. (#3509) Docs: VS Code Extension move to tutorials (#3601) Fix working directory when session has no messages (#3513) ...
Signed-off-by: Soroosh <[email protected]> Signed-off-by: Adam Tarantino <[email protected]>
Previously, when a session had no messages or an invalid working directory, it would default to the home directory. This changes the behavior to use the current working directory instead, which I think is more intuitive and matches user expectations.
Questions
Step to reproduce the problem:
With this Fix
Next time you resume the session created "after applying this fix" will be associated with the correct working directory even if session is empty.