Skip to content

fix: use current working directory as default for session metadata - #2793

Closed
s-soroosh wants to merge 1 commit into
aaif-goose:mainfrom
s-soroosh:fix-working-directory-when-session-has-no-messages
Closed

fix: use current working directory as default for session metadata#2793
s-soroosh wants to merge 1 commit into
aaif-goose:mainfrom
s-soroosh:fix-working-directory-when-session-has-no-messages

Conversation

@s-soroosh

@s-soroosh s-soroosh commented Jun 5, 2025

Copy link
Copy Markdown
Contributor

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

  • Is there any drawback to use cwd as the default working directory instead of home directory?

@s-soroosh
s-soroosh marked this pull request as ready for review June 5, 2025 15:21
@s-soroosh
s-soroosh force-pushed the fix-working-directory-when-session-has-no-messages branch from 1009ed7 to 67e1556 Compare June 5, 2025 15:28
@zanesq
zanesq requested a review from baxen June 18, 2025 20:43
Comment thread crates/goose/src/session/storage.rs Outdated

@michaelneale michaelneale left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

see comment by @DOsinga - looks interesting but I think has to be a default does't there? otherwise it will just fail

@s-soroosh
s-soroosh force-pushed the fix-working-directory-when-session-has-no-messages branch from 67e1556 to 6a4dc5f Compare July 9, 2025 21:13
@s-soroosh
s-soroosh requested a review from michaelneale July 9, 2025 21:14
@s-soroosh

Copy link
Copy Markdown
Contributor Author

@DOsinga @michaelneale Thanks for the reviews! Updated the code to fallback to home dir in case working directory cannot be determined

@michaelneale

Copy link
Copy Markdown
Collaborator

I think CWD make more sense @s-soroosh I like this, but not entirely clear how to test/reproduce, code reads fine, but do you have a scenario to test with?

@michaelneale

Copy link
Copy Markdown
Collaborator

@s-soroosh also will need DCO https://github.com/block/goose/pull/2793/checks?check_run_id=45675312414 it looks like but LGTM otherwise.

@michaelneale

Copy link
Copy Markdown
Collaborator

ie if you could show some samples of it working new way - then I think we are good to go with this for next release

@michaelneale michaelneale added status: in progress p2 Priority 2 - Medium labels Jul 9, 2025
@michaelneale

Copy link
Copy Markdown
Collaborator

actually DCO skipped, this needs to be udpated to main and should pass the build

@michaelneale

Copy link
Copy Markdown
Collaborator

@s-soroosh mind updating this to main branch again?

@michaelneale michaelneale removed the p2 Priority 2 - Medium label Jul 13, 2025
@michaelneale

Copy link
Copy Markdown
Collaborator

closing for now until more feedback

@nyonson

nyonson commented Jul 17, 2025

Copy link
Copy Markdown
Contributor

Just curious what feedback is required for this patch? The issue is definitely a little painpoint in the CLI at the moment.

@s-soroosh

Copy link
Copy Markdown
Contributor Author

Hey, I was busy this week and haven't found time yet to address the latest feedback.
It's mainly about rebasing my branch and write down a scenario to reproduce this issue and how this patch fixes it.
Will work on it later today or tomorrow.

@s-soroosh

s-soroosh commented Jul 18, 2025

Copy link
Copy Markdown
Contributor Author

Reopened a PR with the same changes here.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Goose CLI: Working dir not properly set for new sessions

4 participants