-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix: prioritize session list loading when resuming with -c #5816
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
base: dev
Are you sure you want to change the base?
Conversation
When using 'opencode -c' to resume a session, the session list is now loaded in the blocking phase instead of non-blocking. This eliminates the confusing flash of the dashboard before the resumed session appears. The continue effect now triggers at 'partial' status since the session data is already available at that point.
|
/review |
|
|
||
| await Promise.all(blockingRequests) | ||
| .then(() => { | ||
| if (store.status !== "complete") setStore("status", "partial") |
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.
Suggestion: Consider using spread syntax instead of mutable .push() to stay consistent with the style guide preference of avoiding mutation patterns. This is a minor stylistic improvement - the current code is functional and clear.
| if (store.status !== "complete") setStore("status", "partial") | |
| const blockingRequests: Promise<unknown>[] = [ | |
| sdk.client.config.providers({}, { throwOnError: true }).then((x) => { | |
| batch(() => { | |
| setStore("provider", x.data!.providers) | |
| setStore("provider_default", x.data!.default) | |
| }) | |
| }), | |
| sdk.client.provider.list({}, { throwOnError: true }).then((x) => { | |
| batch(() => { | |
| setStore("provider_next", x.data!) | |
| }) | |
| }), | |
| sdk.client.app.agents({}, { throwOnError: true }).then((x) => setStore("agent", x.data ?? [])), | |
| sdk.client.config.get({}, { throwOnError: true }).then((x) => setStore("config", x.data!)), | |
| ...(args.continue ? [sessionListPromise] : []), | |
| ] | |
| await Promise.all(blockingRequests) |
| async function bootstrap() { | ||
| // blocking | ||
| await Promise.all([ | ||
| const args = useArgs() |
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.
Potential Issue: Calling useArgs() inside an async function (bootstrap) might not be the safest pattern for SolidJS context hooks. Consider moving the useArgs() call to the top of the init function (alongside other hook calls like useExit()), then reference args inside bootstrap. This ensures the hook is called synchronously during component initialization.
| const args = useArgs() | |
| const exit = useExit() | |
| const args = useArgs() | |
| async function bootstrap() { | |
| const sessionListPromise = sdk.client.session.list().then((x) => |
This is a suggestion - the current code may work fine in practice, but the safer pattern would be to call hooks at the top level of the init function.
- Move useArgs() to init top-level for safer SolidJS hook pattern - Use spread syntax instead of .push() to avoid mutation
Summary
opencode -cto resume a sessionChanges
When using
-cflag:session.list()is now loaded in the blocking phase (instead of non-blocking)This ensures the session data is available immediately, so navigation to the resumed session happens before the dashboard ever renders.
Testing
opencode -c