Skip to content

Commit fc7700a

Browse files
code-ashercode-asher
authored
fix: improve reliability of app statuses (#18622)
We were discarding all "working" updates from the screen watcher because we cannot tell the difference between the agent or user changing the screen, but it makes sense to accept it as the very first update, because the agent could be working but neglected to report that fact, so you would never get an initial "working" update (it would just eventually go straight to "idle"). Also messages can start at zero, so I made a fix for that as well, although the first message will be from the LLM and we ignore those anyway, so this probably has no actual effect, but seems more technically correct. And it seems I forgot to actually update the last message ID, which also does not actually matter for user messages (since I think the SSE endpoint will not re-emit a user message it has already emitted), but seems more technically correct to check. Lastly, if we have the screen watcher, ignore the agent's self-reported state and always use "working" since it is unreliable. The idle state will eventually be caught by the watcher.
1 parent ad67733 commit fc7700a

File tree

2 files changed

+390
-225
lines changed

2 files changed

+390
-225
lines changed

‎cli/exp_mcp.go

Lines changed: 60 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -362,11 +362,19 @@ func (*RootCmd) mcpConfigureCursor() *serpent.Command {
362362
}
363363

364364
type taskReport struct {
365-
link string
366-
messageID int64
365+
// link is optional.
366+
link string
367+
// messageID must be set if this update is from a *user* message. A user
368+
// message only happens when interacting via the AI AgentAPI (as opposed to
369+
// interacting with the terminal directly).
370+
messageID *int64
371+
// selfReported must be set if the update is directly from the AI agent
372+
// (as opposed to the screen watcher).
367373
selfReported bool
368-
state codersdk.WorkspaceAppStatusState
369-
summary string
374+
// state must always be set.
375+
state codersdk.WorkspaceAppStatusState
376+
// summary is optional.
377+
summary string
370378
}
371379

372380
type mcpServer struct {
@@ -388,31 +396,48 @@ func (r *RootCmd) mcpServer() *serpent.Command {
388396
return &serpent.Command{
389397
Use: "server",
390398
Handler: func(inv *serpent.Invocation) error {
391-
// lastUserMessageID is the ID of the last *user* message that we saw. A
392-
// user message only happens when interacting via the AI AgentAPI (as
393-
// opposed to interacting with the terminal directly).
394-
var lastUserMessageID int64
395399
var lastReport taskReport
396400
// Create a queue that skips duplicates and preserves summaries.
397401
queue := cliutil.NewQueue[taskReport](512).WithPredicate(func(report taskReport) (taskReport, bool) {
398-
// Use "working" status if this is a new user message. If this is not a
399-
// new user message, and the status is "working" and not self-reported
400-
// (meaning it came from the screen watcher), then it means one of two
401-
// things:
402-
// 1. The AI agent is still working, so there is nothing to update.
403-
// 2. The AI agent stopped working, then the user has interacted with
404-
// the terminal directly. For now, we are ignoring these updates.
405-
// This risks missing cases where the user manually submits a new
406-
// prompt and the AI agent becomes active and does not update itself,
407-
// but it avoids spamming useless status updates as the user is
408-
// typing, so the tradeoff is worth it. In the future, if we can
409-
// reliably distinguish between user and AI agent activity, we can
410-
// change this.
411-
if report.messageID > lastUserMessageID {
412-
report.state = codersdk.WorkspaceAppStatusStateWorking
413-
} else if report.state == codersdk.WorkspaceAppStatusStateWorking && !report.selfReported {
402+
// Avoid queuing empty statuses (this would probably indicate a
403+
// developer error)
404+
if report.state == "" {
414405
return report, false
415406
}
407+
// If this is a user message, discard if it is not new.
408+
if report.messageID != nil && lastReport.messageID != nil &&
409+
*lastReport.messageID >= *report.messageID {
410+
return report, false
411+
}
412+
// If this is not a user message, and the status is "working" and not
413+
// self-reported (meaning it came from the screen watcher), then it
414+
// means one of two things:
415+
//
416+
// 1. The AI agent is not working; the user is interacting with the
417+
// terminal directly.
418+
// 2. The AI agent is working.
419+
//
420+
// At the moment, we have no way to tell the difference between these
421+
// two states. In the future, if we can reliably distinguish between
422+
// user and AI agent activity, we can change this.
423+
//
424+
// If this is our first update, we assume it is the AI agent working and
425+
// accept the update.
426+
//
427+
// Otherwise we discard the update. This risks missing cases where the
428+
// user manually submits a new prompt and the AI agent becomes active
429+
// (and does not update itself), but it avoids spamming useless status
430+
// updates as the user is typing, so the tradeoff is worth it.
431+
if report.messageID == nil &&
432+
report.state == codersdk.WorkspaceAppStatusStateWorking &&
433+
!report.selfReported && lastReport.state != "" {
434+
return report, false
435+
}
436+
// Keep track of the last message ID so we can tell when a message is
437+
// new or if it has been re-emitted.
438+
if report.messageID == nil {
439+
report.messageID = lastReport.messageID
440+
}
416441
// Preserve previous message and URI if there was no message.
417442
if report.summary == "" {
418443
report.summary = lastReport.summary
@@ -600,7 +625,8 @@ func (s *mcpServer) startWatcher(ctx context.Context, inv *serpent.Invocation) {
600625
case agentapi.EventMessageUpdate:
601626
if ev.Role == agentapi.RoleUser {
602627
err := s.queue.Push(taskReport{
603-
messageID: ev.Id,
628+
messageID: &ev.Id,
629+
state: codersdk.WorkspaceAppStatusStateWorking,
604630
})
605631
if err != nil {
606632
cliui.Warnf(inv.Stderr, "Failed to queue update: %s", err)
@@ -650,10 +676,18 @@ func (s *mcpServer) startServer(ctx context.Context, inv *serpent.Invocation, in
650676
// Add tool dependencies.
651677
toolOpts := []func(*toolsdk.Deps){
652678
toolsdk.WithTaskReporter(func(args toolsdk.ReportTaskArgs) error {
679+
// The agent does not reliably report its status correctly. If AgentAPI
680+
// is enabled, we will always set the status to "working" when we get an
681+
// MCP message, and rely on the screen watcher to eventually catch the
682+
// idle state.
683+
state := codersdk.WorkspaceAppStatusStateWorking
684+
if s.aiAgentAPIClient == nil {
685+
state = codersdk.WorkspaceAppStatusState(args.State)
686+
}
653687
return s.queue.Push(taskReport{
654688
link: args.Link,
655689
selfReported: true,
656-
state: codersdk.WorkspaceAppStatusState(args.State),
690+
state: state,
657691
summary: args.Summary,
658692
})
659693
}),

0 commit comments

Comments
 (0)