Skip to content

fix(coderd/provisionerdserver): ensure that SidebarAppID is a known workspace app #19123

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
+327 −186
Closed

fix(coderd/provisionerdserver): ensure that SidebarAppID is a known workspace app #19123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84b852b
chore(coderd/provisionerdserver): remove dangerous usage of dbtestuti…
johnstcn Aug 2, 2025
4e34c89
fix(coderd/provisionerdserver): ensure that SidebarAppID is a known w…
johnstcn Aug 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions coderd/provisionerdserver/provisionerdserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1943,6 +1943,17 @@ func (s *server) completeWorkspaceBuildJob(ctx context.Context, job database.Pro
}
}

var knownAppIDs []string
for _, res := range jobType.WorkspaceBuild.Resources {
for _, agt := range res.Agents {
for _, app := range agt.Apps {
if app.Id == "" {
continue
}
knownAppIDs = append(knownAppIDs, app.Id)
}
}
}
var sidebarAppID uuid.NullUUID
hasAITask := len(jobType.WorkspaceBuild.AiTasks) == 1
if hasAITask {
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also note that this is a hack to get around a fatal error triggered by the bug I described here: #18776 (comment)

// SidebarAppID corresponds to an extant coder_app. If it does not, the
// build will fail with a foreign key constraint violation as workspace_builds.ai_task_sidebar_app_id
// references coder_apps.id.
// Unfortunately, this behavior will result in the workspace build being
// '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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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.",

slog.F("workspace_build_id", workspaceBuild.ID),
slog.F("sidebar_app_id", sidebarAppID),
slog.F("known_app_ids", knownAppIDs),
)
sidebarAppID = uuid.NullUUID{}
hasAITask = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't always trigger on stop, and it sometimes triggers on start transitions too. See #18776 (comment)

}
// Regardless of whether there is an AI task or not, update the field to indicate one way or the other since it
// always defaults to nil. ONLY if has_ai_task=true MUST ai_task_sidebar_app_id be set.
err = db.UpdateWorkspaceBuildAITaskByID(ctx, database.UpdateWorkspaceBuildAITaskByIDParams{
if err = db.UpdateWorkspaceBuildAITaskByID(ctx, database.UpdateWorkspaceBuildAITaskByIDParams{
ID: workspaceBuild.ID,
HasAITask: sql.NullBool{
Bool: hasAITask,
Valid: true,
},
SidebarAppID: sidebarAppID,
UpdatedAt: now,
})
if err != nil {
}); err != nil {
return xerrors.Errorf("update workspace build ai tasks flag: %w", err)
}

Expand Down
Loading
Loading