-
Notifications
You must be signed in to change notification settings - Fork 2.3k
chore: improve timeout for entering password when running goose ui from source #5349
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
DOsinga
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 seems fine, but I wonder in general why we need this at all? it always reeks like either LLM generated defensive code that isn't really useful or is about something that happened at one point (goosed would not start) but not anymore
goosed exiting because of something is much more likely than goosed not starting. and we don't handle the password thing well anyway you say.
|
Good callout. I can look deeper and see if we even still need it. |
|
Is this just in dev mode? I thought I actually just ran into this with a built app when I had it configured for a provider that used oauth. Another fix would be for /status to still return 200 even if we're waiting on a keyring or oauth or something. I'm actually not sure why it wouldn't. Do we not start serving until an agent is created or something? |
|
to put it another way, ideally we don't time out at all if we're waiting for the user to enter something. Maybe for oauth we need to because we don't know if they've closed the browser window, but for the password entry, timing out at all doesn't make much sense |
|
I think this is the password you enter to give goose access to your keychain and the timeout is how long we give goosed to become responsive |
|
Good questions @jamadeo and I agree. I think it comes up more in development for two reasons I think:
But totally possible this bug exists in the built version of the app, which is actually quite rough... I'll merge this to at least make it better for the moment and look closer at whether |
right, yeah, but the combination of those effectively means you have X seconds to enter your password or the app dies, if I'm understanding this issue the app should die if goosed doesn't start up, but it should not die if it's just waiting for user input |
yes agree. I've made it better in this PR as we now have a two minute timebomb instead of a 20s timebomb, but haven't fixed it. I'm unclear if it happens the same way in a built version of goose though. Will test now. |
|
huh, actually I'm also unable to replicate. if I start a newly built goosed, I get a "200 ok" from /status before it asks for keyring access |
|
but the user input request comes from goosed, so the electron app has no way to see if it is that. on the other hand, why does the /status handler not succeed? when I tested it, /status returns with the keychain box open, so when exactly do you see this error? |
|
I don't see this - running from the 1.12.0 branch |
|
Attaching what I experience on goose-20s-timeout.mov |
|
yeah, something is off here, I can reproduce it when I run |
|
Ok, this is because as of #4684 we configure a provider using GOOSE_DEFAULT_PROVIDER / GOOSE_DEFAULT_MODEL before starting the server. This could block on oauth or password entry |
…om source (block#5349) Signed-off-by: Blair Allan <[email protected]>
* main: fixing typo in blog metadata (#5382) feat: add new blog entry on adopting Goose in the enterprise (#5381) Blog/acp intro oct 2024 (#5379) feat: add A/B test framework generator recipe (#5378) Doc: (blog) - Deep Dive into goose's Extension System and Model Context Protocol (MCP) (#5291) Some system prompt tidying (#5313) Fix scheduler jobs dates formatting (#5368) Use Instructions as Prompt in Scheduler (#5359) feat(snowflake): add support for newer Claude 4.5 and 4 models (#5350) Add bottom menu extension selection (#5352) (re)Standardize Session Name Attribute (#5279) chore: improve timeout for entering password when running goose ui from source (#5349)
* main: fixing typo in blog metadata (#5382) feat: add new blog entry on adopting Goose in the enterprise (#5381) Blog/acp intro oct 2024 (#5379) feat: add A/B test framework generator recipe (#5378) Doc: (blog) - Deep Dive into goose's Extension System and Model Context Protocol (MCP) (#5291) Some system prompt tidying (#5313) Fix scheduler jobs dates formatting (#5368) Use Instructions as Prompt in Scheduler (#5359) feat(snowflake): add support for newer Claude 4.5 and 4 models (#5350) Add bottom menu extension selection (#5352) (re)Standardize Session Name Attribute (#5279) chore: improve timeout for entering password when running goose ui from source (#5349)
* main: (54 commits) fixing typo in blog metadata (#5382) feat: add new blog entry on adopting Goose in the enterprise (#5381) Blog/acp intro oct 2024 (#5379) feat: add A/B test framework generator recipe (#5378) Doc: (blog) - Deep Dive into goose's Extension System and Model Context Protocol (MCP) (#5291) Some system prompt tidying (#5313) Fix scheduler jobs dates formatting (#5368) Use Instructions as Prompt in Scheduler (#5359) feat(snowflake): add support for newer Claude 4.5 and 4 models (#5350) Add bottom menu extension selection (#5352) (re)Standardize Session Name Attribute (#5279) chore: improve timeout for entering password when running goose ui from source (#5349) Fix legacy import (#5343) Unify loading goose messages and usechatstream determines chat state (#5306) Docs: goose session export and goose session import (#5267) Create recipe dir on save (#5337) docs: Update Discord link (#5335) [recipe workflow]: Fix `Invalid revision range` error (#5334) Add tech-article-explainer recipe (#5333) doc: added beta banner for old blog post (#5332) ...
…om source (block#5349) Signed-off-by: Blair Allan <[email protected]>

Right now we only have 20sec to enter our password when we run goose ui locally and goosed starts and needs the keychain password. I often miss it and goose turns into a brick, and needs to be relaunched. This will give 2min instead of 20sec.