fix(coderd/provisionerdserver): ensure that SidebarAppID is a known workspace app #19123
Conversation
…l.DisableForeignKeysAndTriggers
There was a problem hiding this comment.
I reproduced the issue and can confirm that this fix allows the workspace build to complete. I'm concerned, though, that it hides the error from the user. We have a report from a user who ran into this during the start transition, and with this fix, the task will immediately disappear from the UI list after creation, which will be confusing. Still, I suppose that's better than leaving the workspace in a failed, stuck state.
| @@ -1959,18 +1970,33 @@ func (s *server) completeWorkspaceBuildJob(ctx context.Context, job database.Pro | |||
| sidebarAppID = uuid.NullUUID{UUID: id, Valid: true} | |||
| } | |||
|
|
|||
| // NOTE(Cian): If stopping a workspace build, we must ensure that the | |||
There was a problem hiding this comment.
I'd also note that this is a hack to get around a fatal error triggered by the bug I described here: #18776 (comment)
| // 'silently' not configured as an AI task except for a warning in the logs. | ||
| // Ideally, we should also accept an app slug as these are less likely to change. | ||
| if sidebarAppID.Valid && !slices.Contains(knownAppIDs, sidebarAppID.UUID.String()) { | ||
| s.Logger.Warn(ctx, "workspace build has unknown sidebar app ID", |
There was a problem hiding this comment.
I'd reference the issue in the log. A user has reported this problem also occurring during the start transition. With this fix, after a user spawns a task, they'll see it immediately disappear after creation and will wonder where it went. When they look at server logs, let's explain the problem and point them where to look next.
| s.Logger.Warn(ctx, "workspace build has unknown sidebar app ID", | |
| s.Logger.Error(ctx, "task creation failed. Workspace build has unknown sidebar app ID - this is a known issue. See https://github.com/coder/coder/issues/18776 for a workaround.", |
|
It doesn't seem right that our database schema requires a sidebar app ID to STOP the workspace. Generally on a STOP, you don't have any agents, meaning you won't have any Also, what's the overall game plan for this "workaround"? It doesn't seem like a permanent solution. Is the idea that we will patch and then get a proper solution going? Do we have a sense for how long a proper solution will take? I.e. can we just do the real fix without mucking around with a patch release? |
There was a problem hiding this comment.
I think this is going to be noticable to everyone using AI Tasks in a bad way. See inline comment.
This issue comment sketches out what I think the correct fix is: #18776 (comment)
| slog.F("known_app_ids", knownAppIDs), | ||
| ) | ||
| sidebarAppID = uuid.NullUUID{} | ||
| hasAITask = false |
There was a problem hiding this comment.
I think this will trigger on basically every workspace stop, since generally speaking, stop transitions won't have any agents. So, every time you stop an AI Task it will disappear.
Has anyone tested this out end to end? Am I wrong about this always triggering on stop?
There was a problem hiding this comment.
It doesn't always trigger on stop, and it sometimes triggers on start transitions too. See #18776 (comment)
|
Will investigate an alternative solution that does not result in silently unsetting the |
Relates to #18776
Applies a quick and dirty 'workaround' when the sidebar app ID references an unknown app ID for the build.