- Notifications
You must be signed in to change notification settings - Fork1k
fix: handle chat app not found#19947
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
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
BrunoQuaresma commentedSep 25, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@johnstcn I’ve added support for unhealthy apps 🙏 — thanks a lot for catching that! |
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.
LGTM, just some potential enhancements for troubleshooting misbehaving workspace apps in the copy!
| App unhealthy | ||
| </h3> | ||
| <spanclassName="text-content-secondary text-sm"> | ||
| Check the logs for details | ||
| </span> |
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: add some more details so it's clear what the relevant app is, e.g.
| Appunhealthy | |
| </h3> | |
| <spanclassName="text-content-secondary text-sm"> | |
| Checkthelogsfordetails | |
| </span> | |
| App"${app.name}"unhealthy | |
| </h3> | |
| <spanclassName="text-content-secondary text-sm"> | |
| Herearesometroubleshootingstepsyoucantake: | |
| <ul> | |
| <li>Try running the following inside your workspce:<code>curl -v "${app.healthcheck.url}"</code></li> | |
| <li>Check<code>/tmp/coder-agent.log</code> inside your workspace "${workspace.name}" for more information.</li> | |
| </ul> | |
| </span> |
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.
Updated using your suggestion as the base:
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.
mostly looks good, I think cian is right about app health being valuable tho
| constapps=getTaskApps(task).filter( | ||
| // The Chat UI app will be displayed in the sidebar, so we don't want to | ||
| // show it as a web app. | ||
| (app)=>app.id!==task.workspace.latest_build.ai_task_sidebar_app_id, | ||
| ); |
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.
I think agetTaskApp helper could still be nice here
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.
Not sure if I understood—getTaskApps is already being used 🤔
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.
sorry, I meantgetChatApp
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.
and you're right, it's doing the opposite here 🤦♀️
| constchatApp=getTaskApps(task).find( | ||
| (app)=>app.id===task.workspace.latest_build.ai_task_sidebar_app_id, | ||
| ); |
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.
...and could be used here as well
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.
They’re quite different:
- The first removes the chat app from the apps list.
- The second fetches the chat app.
The only common part is accessingai_task_sidebar_app_id, so a helper could be:
functiongetChatAppId(task:Task){returntask.workspace.latest_build.ai_task_sidebar_app_id;}
That said, I don’t see much value in adding this helper right now—what do you think?
@asilac I’m merging this to help us identify some template issues in dogfood, but I’ll keep an eye on your comments and address any concerns or improvements in a follow-up PR. |
c2d5143 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Sometimes users can misconfigure the app used for chat. When that happens, we should make it clear to the user.
TaskAppIframehandle all task iframes so we don’t need a separate iframe component for chat.