feat: add notification for task status by ssncferreira · Pull Request #19965 · coder/coder

ssncferreira · 2025-09-25T15:54:06Z

Description

Send a notification to the workspace owner when an AI task’s app state becomes Working or Idle.
An AI task is identified by a workspace build with HasAITask = true and AITaskSidebarAppID matching the agent app’s ID.

Changes

Add TemplateTaskWorking notification template.
Add TemplateTaskIdle notification template.
Add GetLatestWorkspaceAppStatusesByAppID SQL query to get the workspace app statuses ordered by latest first.
Update PATCH /workspaceagents/me/app-status to enqueue:
- TemplateTaskWorking when state transitions to working
- TemplateTaskIdle when state transitions to idle
Notification labels include:
- task: task initial prompt
- workspace: workspace name
Notification dedupe: include a minute-bucketed timestamp (UTC truncated to the minute) in the enqueue data to allow identical content to resend within the same day (but not more than once per minute).

Closes: #19776

ssncferreira · 2025-09-25T17:46:38Z

+	enabled_by_default
+) VALUES (
+			 'bd4b7168-d05e-4e19-ad0f-3593b77aa90f',
+			 'Task Working',


I kept the terminology aligned with the current workspace agent app status (Working, Idle). In PR: https://github.com/coder/coder/pull/19773/files#diff-1894d4f2ccb1125739e504fc6d8d3a7daeec9dd829a81c342cf28f3e92efbf58R2-R7 I see the task status types differ. Is that finalized? If yes, should I update this PR to adopt the new task status terms?

ssncferreira · 2025-09-25T17:48:22Z

+		if workspaceBuild.HasAITask.Valid && workspaceBuild.HasAITask.Bool &&
+			workspaceBuild.AITaskSidebarAppID.Valid && workspaceBuild.AITaskSidebarAppID.UUID == app.ID {


With the current data model, this is the correct way to determine if a workspace agent app is an AI task, right?

For now, yes. This will need to be refactored later.

johnstcn · 2025-09-26T08:41:13Z

+		if workspaceBuild.HasAITask.Valid && workspaceBuild.HasAITask.Bool &&
+			workspaceBuild.AITaskSidebarAppID.Valid && workspaceBuild.AITaskSidebarAppID.UUID == app.ID {


For now, yes. This will need to be refactored later.

johnstcn · 2025-09-26T09:11:10Z

+				notificationTemplate = notifications.TemplateTaskIdle
+			}
+
+			if _, err := api.NotificationsEnqueuer.EnqueueWithData(


Before we enqueue a notification, we should ensure that the new task status is different to the previous. An agent could insert multiple app statuses with the same state but different messages e.g.

"working / doing thing A"

"working / doing thing B"

Ok, that is good to know. I was not aware it was possible for a task to transition to the same status. Addressed in 06ed8c7

mafredri

Looks good to me, nice work 👍🏻. Not much to add from what Cian already, just one question/potential gotcha.

mafredri · 2025-09-26T11:42:31Z

+					// Include a minute-bucketed timestamp to bypass per-day dedupe,
+					// allowing identical content to resend within the same day
+					// (but not more than once per minute)
+					"dedupe_bypass_ts": api.Clock.Now().UTC().Truncate(time.Minute),


How does this work if the task quickly jumps between working/idle/working? Is there a chance we miss the second working and never emit it? If yes, I think that would lead to bad UX as the notifications are untrustworthy.

If a task were to go through the state transition Idle -> Working -> Idle -> Working -> Idle all within the space of a minute, you'd only get the initial -> Working and -> Idle notifications.

Thanks for confirming, that's what I suspected. It gets a bit funky when we're bordering on the minute mark.

14:39:00 Working
14:39:29 Idle
14:39:59 Working
14:40:30 Idle

We get Working -> Idle -> Idle (alt. Idle -> Working -> Working).

Yes, that’s a good point. There are still scenarios where the system could skip some notification types, resulting in the same type being sent back-to-back. The minute-level dedupe was intended as a safeguard to avoid spamming users. Based on the problem definition "By definition tasks may take quite a long time to be completed. We do not want the users to stare at the screen waiting because that invalidates the whole purpose." the assumption was that these transitions wouldn’t happen very quickly.

Instead of truncating to the minute, should we use the exact timestamp? That would avoid missing intermediate states, but it does increase the risk of too many notifications. Open to suggestions 😊

Unfortunately I think there is no perfect solution here. We could still truncate to the second, but like before, any truncation runs the risk of missing a notification. I'd be satisfied with truncating it down to the second. @mafredri thoughts?

I'd rather risk missing some notifications than spamming and potentially overloading coderd or whatever notification backed is in use.

Second or even 5-10 second truncate would work. 👍🏻 Especially if we implement the safeguard that Cian mentioned, i.e. previous status must have been different from current status, then we minimize the risk of spam.

Added a 10-second truncate to the dedupe key and a check to send notifications only on new transitions.
Addressed in 06ed8c7

…ks-notifications

johnstcn

Some comments below, but I don't need to review again.

johnstcn · 2025-09-29T10:04:53Z

Obligatory reminder to check migration number before merge!

johnstcn · 2025-09-29T10:23:20Z

+-- name: GetLatestWorkspaceAppStatusesByAppID :many
+SELECT *
+FROM workspace_app_statuses
+WHERE app_id = @app_id::uuid
+ORDER BY created_at DESC, id DESC;
+


Would it make sense to instead add an ORDER BY clause to GetWorkspaceAppStatusesByAppIDs?

We could add ORDER BY to GetWorkspaceAppStatusesByAppIDs and rename it to reflect the ordering (e.g., GetLatestWorkspaceAppStatusesByAppIDs). For this PR I only need one app, newest-first, so the dedicated query seemed to keep things clearer.

…ks-notifications

DanielleMaywood

Approve with a non-blocking issue:

DanielleMaywood · 2025-09-29T14:48:34Z

 }

+func (q *querier) GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]database.WorkspaceAppStatus, error) {
+	if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {


I realize the other functions around here do the same but I think we were wanting to stop introducing more uses of ResourceSystem

AFAIK, there isn’t a dedicated resource for workspace apps, so operations on them currently authorize against rbac.ResourceSystem. I’m not sure what the best alternative is, skipping the authorization check would make this data accessible to anyone, which we don’t want.

This is unfortunately a pre-existing issue from the introduction of app statuses. It may be worth addressing in a follow-up.

github-actions Bot assigned ssncferreira Sep 25, 2025

feat: add notification for task status

b20b801

ssncferreira force-pushed the ssncferreira/feat-tasks-notifications branch from 62ff41a to b20b801 Compare September 25, 2025 17:37

ssncferreira commented Sep 25, 2025

View reviewed changes

ssncferreira marked this pull request as ready for review September 25, 2025 17:49

ssncferreira requested review from DanielleMaywood, johnstcn and mafredri September 25, 2025 17:49

johnstcn reviewed Sep 26, 2025

View reviewed changes

mafredri reviewed Sep 26, 2025

View reviewed changes

ssncferreira added 5 commits September 26, 2025 18:55

chore: address comments

06ed8c7

Merge remote-tracking branch 'origin/main' into ssncferreira/feat-tas…

b433146

…ks-notifications

chore: fix database migration

1503e05

fix: add GetLatestWorkspaceAppStatusesByAppID test to dbauthz

5bcc9dc

Merge remote-tracking branch 'origin/main' into ssncferreira/feat-tas…

a33540d

…ks-notifications

ssncferreira requested review from johnstcn and mafredri September 29, 2025 10:00

johnstcn approved these changes Sep 29, 2025

View reviewed changes

ssncferreira mentioned this pull request Sep 29, 2025

feat(site): add task notifications to user settings #20006

Merged

ssncferreira added 3 commits September 29, 2025 14:44

chore: address comments

6bd39ec

Merge remote-tracking branch 'origin/main' into ssncferreira/feat-tas…

e75e648

…ks-notifications

fix: database migration

74dc3ee

DanielleMaywood approved these changes Sep 29, 2025

View reviewed changes

ssncferreira merged commit fdb0267 into main Sep 29, 2025

ssncferreira deleted the ssncferreira/feat-tasks-notifications branch September 29, 2025 15:44

github-actions Bot locked and limited conversation to collaborators Sep 29, 2025

		if workspaceBuild.HasAITask.Valid && workspaceBuild.HasAITask.Bool &&
		workspaceBuild.AITaskSidebarAppID.Valid && workspaceBuild.AITaskSidebarAppID.UUID == app.ID {

Conversation

ssncferreira commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mafredri left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mafredri Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnstcn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DanielleMaywood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ssncferreira commented Sep 25, 2025 •

edited

Loading

mafredri Sep 26, 2025 •

edited

Loading