- Notifications
You must be signed in to change notification settings - Fork1k
feat: display startup script logs while agent is starting#19530
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Looks good! Just had a few small suggestions
| level:"info", | ||
| output:line, | ||
| source_id:MockWorkspaceAgentLogSource.id, | ||
| created_at:newDate().toISOString(), |
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.
Just to be on the safe side, do we want to hard-code the date here, sincenew Date() would create a non-deterministic value for each story run?
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.
Good catch!
site/src/pages/TaskPage/TaskPage.tsx Outdated
| Your task will be running in a few moments | ||
| </div> | ||
| </header> | ||
| <sectionclassName="flex-1 p-16 overflow-y-auto"> |
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.
Is there a reason for theflex-1? I wouldn't expect a page-level component to need to worry about arbitrarily growing or shrinking
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.
That is true
site/src/pages/TaskPage/TaskPage.tsx Outdated
| <h3className="m-0 font-medium text-content-primary text-xl"> | ||
| Starting your workspace | ||
| </h3> | ||
| <divclassName="text-content-secondary"> |
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.
Could we swap thisdiv for ap?
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.
Sure!
| useEffect(()=>{ | ||
| if(listRef.current){ | ||
| listRef.current.scrollToItem(logs.length-1,"end"); |
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.
Do you know what the scroll behavior is for the React Window component? As in, do you know if the scroll is instant, or if it does an animation?
Because if it's instant, we could swap inuseLayoutEffect to reduce any risk of screen flickering
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.
It is instant, and we can move to useuseLayoutEffect for sure.
site/src/pages/TaskPage/TaskPage.tsx Outdated
| <h3className="m-0 font-medium text-content-primary text-xl"> | ||
| Running startup scripts | ||
| </h3> | ||
| <divclassName="text-content-secondary"> |
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.
Think we could use ap here, too
…agent-being-ready-for-tasks
…agent-being-ready-for-tasks
59525f8 intomainUh oh!
There was an error while loading.Please reload this page.
Closes#19363
Screenshot:
Demo:
Screen.Recording.2025-08-22.at.16.00.23.mov