Skip to content

Conversation

@s-soroosh
Copy link
Contributor

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

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
Copy link
Collaborator

@michaelneale michaelneale left a comment

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
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
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
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
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
Collaborator

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

@michaelneale
Copy link
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
Collaborator

closing for now until more feedback

@nyonson
Copy link
Contributor

nyonson commented Jul 17, 2025

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
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
Copy link
Contributor Author

s-soroosh commented Jul 18, 2025

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