Add proper binary guards for authorize_refresh/1

go/deployment-operator/pkg/agentrun-harness/controller/analysis_gate_test.go

@@ -405,6 +405,88 @@ func TestBuildBabysitContextIncludesDelayedNewComments(t *testing.T) {
 	require.Contains(t, bCtx.Prompt, "review:2")
 }
 
+func TestBuildBabysitContextSkipsStateChangesWithoutCommentsOrFailingCI(t *testing.T) {
+	t.Parallel()
+
+	prURL := "https://github.com/pluralsh/console/pull/1"
+	in := &agentRunController{dir: t.TempDir()}
+	agentRun := &gqlclient.AgentRunFragment{
+		PullRequests: []*gqlclient.PullRequestFragment{{URL: prURL}},
+	}
+	client := &fakeBabysitGRPCClient{
+		details: map[string]*scm.PRDetails{
+			prURL: {
+				Title:   "Fix babysit",
+				HeadRef: "fix/babysit",
+				State:   scm.PRStateOpen,
+				CIChecks: []scm.CICheck{{
+					Name:       "test",
+					Status:     scm.CICheckStatusCompleted,
+					Conclusion: scm.CICheckConclusionSuccess,
+				}},
+			},
+		},
+	}
+
+	require.Nil(t, in.buildBabysitContext(context.Background(), agentRun, client))
+	baselineSHA := in.lastPRSHA
+
+	client.details[prURL] = &scm.PRDetails{
+		Title:   "Fix babysit docs",
+		HeadRef: "fix/babysit",
+		State:   scm.PRStateOpen,
+		CIChecks: []scm.CICheck{{
+			Name:   "test",
+			Status: scm.CICheckStatusInProgress,
+		}},
+	}
+
+	require.Nil(t, in.buildBabysitContext(context.Background(), agentRun, client))
+	require.NotEqual(t, baselineSHA, in.lastPRSHA)
+}
+
+func TestBuildBabysitContextIncludesFailingCIWithoutComments(t *testing.T) {
+	t.Parallel()
+
+	prURL := "https://github.com/pluralsh/console/pull/1"
+	in := &agentRunController{dir: t.TempDir()}
+	agentRun := &gqlclient.AgentRunFragment{
+		PullRequests: []*gqlclient.PullRequestFragment{{URL: prURL}},
+	}
+	client := &fakeBabysitGRPCClient{
+		details: map[string]*scm.PRDetails{
+			prURL: {
+				Title:   "Fix babysit",
+				HeadRef: "fix/babysit",
+				State:   scm.PRStateOpen,
+				CIChecks: []scm.CICheck{{
+					Name:       "test",
+					Status:     scm.CICheckStatusCompleted,
+					Conclusion: scm.CICheckConclusionSuccess,
+				}},
+			},
+		},
+	}
+
+	require.Nil(t, in.buildBabysitContext(context.Background(), agentRun, client))
+
+	client.details[prURL] = &scm.PRDetails{
+		Title:   "Fix babysit",
+		HeadRef: "fix/babysit",
+		State:   scm.PRStateOpen,
+		CIChecks: []scm.CICheck{{
+			Name:       "test",
+			Status:     scm.CICheckStatusCompleted,
+			Conclusion: scm.CICheckConclusionFailure,
+		}},
+	}
+
+	bCtx := in.buildBabysitContext(context.Background(), agentRun, client)
+	require.NotNil(t, bCtx)
+	require.Contains(t, bCtx.Prompt, "Failing CI checks")
+	require.Contains(t, bCtx.Prompt, "test")
+}
+
 func TestBuildBabysitContextIncludesReviewSummariesAsNonReactable(t *testing.T) {
 	t.Parallel()
 

go/deployment-operator/pkg/agentrun-harness/controller/controller.go

@@ -384,6 +384,13 @@ func (in *agentRunController) buildBabysitContext(ctx context.Context, agentRun
 		return nil
 	}
 
+	if !hasBabysitActivity(enriched) {
+		in.lastPRSHA = sha
+		in.lastPRCheckAt = time.Now()
+		klog.V(log.LogLevelExtended).InfoS("PR state changed without new comments or failing CI, skipping reprompt")
+		return nil
+	}
+
 	// Determine working branch from the live PR head ref.
 	// Fall back to a generic placeholder if none of the PRs carry a head ref.
 	branch := "your working branch"
@@ -479,6 +486,30 @@ func buildBabysitPrompt(branch, _ string, prs []toolv1.EnrichedPR, lastChecked t
 	return sb.String()
 }
 
+func hasBabysitActivity(prs []toolv1.EnrichedPR) bool {
+	for _, pr := range prs {
+		if len(pr.NewComments) > 0 {
+			return true
+		}
+		if pr.Details != nil && hasFailingCICheck(pr.Details.CIChecks) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func hasFailingCICheck(checks []scm.CICheck) bool {
+	for _, ci := range checks {
+		switch ci.Conclusion {
+		case scm.CICheckConclusionFailure, "timed_out", scm.CICheckConclusionCancelled:
+			return true
+		}
+	}
+
+	return false
+}
+
 func (in *agentRunController) newOrUpdatedPRComments(prURL string, comments []scm.PRComment) []scm.PRComment {
 	in.ensureSeenPRCommentBodies()
 

lib/console/chat/utils.ex

@@ -31,11 +31,19 @@ defmodule Console.Chat.Utils do
   """
   def handle_mention(%Reference{id: external_id} = msg, %Reference{text: channel} = chan_ref, %ChatConnection{id: id} = conn, %{} = extra) do
     bot = Workbenches.workbench_chatbot(id, channel) |> Repo.preload([user: [:groups]])
-    with %WorkbenchChatbot{user: %User{} = user, prompt: prompt, message_behavior: behavior} = chatbot <- bot do
-      prompt = prompt(chat: conn, msg: msg, channel: chan_ref, custom: prompt, behavior: behavior)
+    with %WorkbenchChatbot{user: %User{} = user, prompt: custom, message_behavior: behavior} = chatbot <- bot do
       case parent_job(msg) do
-        %WorkbenchJob{} = job -> Workbenches.create_message(%{prompt: prompt}, job, user)
+        %WorkbenchJob{} = job ->
+          prompt = reply_prompt(chat: conn, msg: msg, channel: chan_ref, custom: custom, job: job)
+
+          Workbenches.create_queued_prompt(%{
+            prompt: prompt,
+            dequeable_at: DateTime.utc_now()
+          }, job, user)
+
         _ ->
+          prompt = prompt(chat: conn, msg: msg, channel: chan_ref, custom: custom, behavior: behavior)
+
           chatbot_message =
             Map.merge(%{
               message: msg.text,
@@ -61,7 +69,7 @@ defmodule Console.Chat.Utils do
     chatbot_msg(id)
     |> Repo.preload(:workbench_job, force: true)
     |> case do
-      %ChatbotMessage{workbench_job: %WorkbenchJob{status: s} = job} when s not in ~w(running pending)a -> job
+      %ChatbotMessage{workbench_job: %WorkbenchJob{} = job} -> job
       _ -> nil
     end
   end
@@ -71,4 +79,5 @@ defmodule Console.Chat.Utils do
   defp cache_id(%ChatbotMessage{id: id}), do: id
 
   EEx.function_from_file(:defp, :prompt, Console.priv_filename(["prompts", "workbench", "chat.md.eex"]), [:assigns])
+  EEx.function_from_file(:defp, :reply_prompt, Console.priv_filename(["prompts", "workbench", "chat_reply.md.eex"]), [:assigns])
 end

lib/console/services/users.ex

@@ -258,14 +258,15 @@ defmodule Console.Services.Users do
   Determines if a user can refresh their jwt and returns the user back if so
   """
   @spec authorize_refresh(binary) :: user_resp
-  def authorize_refresh(token) do
+  def authorize_refresh(token) when is_binary(token) do
     get_refresh_token(token)
     |> Repo.preload([:user])
     |> case do
       %RefreshToken{user: user} -> {:ok, user}
       _ -> {:error, "could not fetch refresh token"}
     end
   end
+  def authorize_refresh(_), do: {:error, "no refresh token provided"}
 
   def create_service_account(attrs) do
     %User{service_account: true}

priv/prompts/workbench/chat_reply.md.eex

@@ -0,0 +1,5 @@
+A followup message (id=<%= @msg.id %>) was posted in the <%= @channel.text %> (id=<%= @channel.id %>) channel:
+
+<%= @msg.text %>
+
+React to this followup and use it to continue the existing job.

priv/prompts/workbench/job.md.eex

@@ -39,7 +39,11 @@ The overarching task is as follows:
 
 <%= @prompt %>
 
-The user might resteer this task, so if you see any user messages, be sure to use them to guide what work needs to be done as well.
+There can be follow-up messages to this job, which can redefine the task at hand.  You should use them to guide what work needs to be done as well.  Examples of this are:
+
+* a user starting a job as an analysis task and then realizing they want to create a pr or make some change in response to findings.
+* a user doing additional work in the course of a long running job.
+* etc
 
 ## Tone of Voice Guidance
 

test/console/chat/utils_test.exs

@@ -2,7 +2,7 @@ defmodule Console.Chat.UtilsTest do
   use Console.DataCase, async: true
 
   alias Console.Chat.{Reference, Utils}
-  alias Console.Schema.{ChatbotMessage, WorkbenchJob, WorkbenchJobActivity}
+  alias Console.Schema.{ChatbotMessage, QueuedPrompt, WorkbenchJob, WorkbenchJobActivity}
 
   describe "handle_mention/3" do
     test "creates a workbench job for a new chatbot mention" do
@@ -40,7 +40,7 @@ defmodule Console.Chat.UtilsTest do
       refute chatbot_msg.external_parent_id
     end
 
-    test "creates a job message for a reply to a prior chatbot message" do
+    test "queues an immediate prompt for a reply to a prior chatbot message" do
       user = insert(:user)
       workbench = insert(:workbench, read_bindings: [%{user_id: user.id}])
       conn = insert(:chat_connection)
@@ -73,15 +73,63 @@ defmodule Console.Chat.UtilsTest do
         text: "<@bot> follow up on this"
       }
 
-      {:ok, %WorkbenchJobActivity{} = activity} =
+      {:ok, %QueuedPrompt{} = prompt} =
         Utils.handle_mention(msg, %Reference{id: "C123", text: channel}, conn)
 
-      assert activity.workbench_job_id == job.id
-      assert activity.user_id == user.id
-      assert activity.type == :user
-      assert activity.status == :successful
-      assert activity.prompt =~ msg.text
-      assert refetch(job).status == :pending
+      assert prompt.workbench_job_id == job.id
+      assert prompt.user_id == user.id
+      assert prompt.prompt =~ msg.text
+      assert prompt.prompt =~ "A followup message (id=#{msg.id})"
+      assert prompt.prompt =~ "React to this followup and use it to continue the existing job"
+      refute prompt.prompt =~ "You should reply to the original message"
+      assert DateTime.compare(prompt.dequeable_at, DateTime.utc_now()) in [:lt, :eq]
+      assert refetch(job).status == :successful
+      assert Repo.all(WorkbenchJobActivity) == []
+    end
+
+    test "queues replies even when the parent job is active" do
+      user = insert(:user)
+      workbench = insert(:workbench, read_bindings: [%{user_id: user.id}])
+      conn = insert(:chat_connection)
+      channel = "chat-channel"
+      parent_ts = "1730000000.000001"
+
+      insert(:workbench_chatbot,
+        channel: channel,
+        chat_connection: conn,
+        workbench: workbench,
+        user: user
+      )
+
+      job =
+        insert(:workbench_job,
+          status: :running,
+          user: user,
+          workbench: workbench
+        )
+
+      insert(:chatbot_message,
+        external_id: parent_ts,
+        chat_connection: conn,
+        workbench_job: job
+      )
+
+      msg = %Reference{
+        id: "1730000001.000002",
+        parent_id: parent_ts,
+        text: "<@bot> follow up while running"
+      }
+
+      {:ok, %QueuedPrompt{} = prompt} =
+        Utils.handle_mention(msg, %Reference{id: "C123", text: channel}, conn)
+
+      assert prompt.workbench_job_id == job.id
+      assert prompt.prompt =~ msg.text
+      assert prompt.prompt =~ "A followup message (id=#{msg.id})"
+      assert prompt.prompt =~ "React to this followup and use it to continue the existing job"
+      refute prompt.prompt =~ "You should reply to the original message"
+      assert DateTime.compare(prompt.dequeable_at, DateTime.utc_now()) in [:lt, :eq]
+      assert refetch(job).status == :running
     end
   end
 end