From 46b76ccf0dff2622aba85196a583c4028671b841 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Fri, 7 Aug 2026 18:04:13 -0700 Subject: [PATCH 01/15] feat(tasks): accept side_question commands on task runs Adds side_question to the task-run command channel: the serializer validates a non-empty question, and the view forwards the command to the sandbox agent server like other sandbox control commands. side_question requires Code access (has_tasks_access): it drives the agent and spends model tokens on the caller's behalf. user_message stays ungated so the Inbox Discuss flow keeps working. Nothing calls this yet. The desktop /btw command that uses it ships separately, so a client on the old build is unaffected and a sandbox predating the command rejects it with an error the caller sees. Generated-By: PostHog Code Task-Id: 9086ddae-4430-49ef-b956-66f90f356b00 --- .../tasks/backend/presentation/serializers.py | 4 ++ .../tasks/backend/presentation/views/api.py | 12 ++++- products/tasks/backend/tests/test_api.py | 49 +++++++++++++++++++ .../tasks/frontend/generated/api.schemas.ts | 5 +- products/tasks/frontend/generated/api.ts | 2 +- products/tasks/frontend/generated/api.zod.ts | 7 +-- services/mcp/src/api/generated.ts | 5 +- 7 files changed, 77 insertions(+), 7 deletions(-) diff --git a/products/tasks/backend/presentation/serializers.py b/products/tasks/backend/presentation/serializers.py index 4bb59c291151..68365addfb50 100644 --- a/products/tasks/backend/presentation/serializers.py +++ b/products/tasks/backend/presentation/serializers.py @@ -2991,6 +2991,7 @@ class TaskRunCommandRequestSerializer(serializers.Serializer): "pi/rpc", "queue_get", "queue_clear", + "side_question", ] # Cap on the serialized mcp_response params (docs/CLOUD-MCP-RELAY.md): the relayed JSON-RPC @@ -3063,6 +3064,9 @@ def validate(self, attrs): raise serializers.ValidationError( {"params": "user_message requires a non-empty content string, artifact_ids, or both"} ) + elif method == "side_question": + self._require_nonempty_string(params, "question") + params["question"] = params["question"].strip() elif method == "pi/rpc": command = params.get("command") if not isinstance(command, dict): diff --git a/products/tasks/backend/presentation/views/api.py b/products/tasks/backend/presentation/views/api.py index 41cdf45f0900..3942cc4dedf2 100644 --- a/products/tasks/backend/presentation/views/api.py +++ b/products/tasks/backend/presentation/views/api.py @@ -2125,7 +2125,7 @@ def stream_token(self, request, pk=None, **kwargs): summary="Send command to task run", description="Queue user_message JSON-RPC commands through the task workflow and forward sandbox control " "commands to the agent server. Supports user_message, cancel, close, permission_response, " - "set_config_option, mcp_response, native Pi RPC commands, and Pi queue operations.", + "set_config_option, mcp_response, side_question, native Pi RPC commands, and Pi queue operations.", strict_request_validation=True, ) @action( @@ -2159,6 +2159,16 @@ def command(self, request, pk=None, **kwargs): request_id = request.validated_data.get("id") params = request.validated_data.get("params") + # Unlike user_message (deliberately ungated below so Inbox "Discuss" works), + # side_question is only surfaced by the Desktop client, so it can require Code access. + if method == "side_question" and not tasks_access.has_tasks_access(request.user): + return Response( + TaskRunErrorResponseSerializer( + {"error": "side_question requires PostHog Code access.", "code": "code_access_required"} + ).data, + status=status.HTTP_403_FORBIDDEN, + ) + if method == "user_message": # The Inbox starts interactive runs and drops the user straight into this composer, # so "Discuss" and scout chat replies must not require Desktop access. Their tasks diff --git a/products/tasks/backend/tests/test_api.py b/products/tasks/backend/tests/test_api.py index f69846fb53f3..3bab10615858 100644 --- a/products/tasks/backend/tests/test_api.py +++ b/products/tasks/backend/tests/test_api.py @@ -9820,6 +9820,55 @@ def test_command_proxies_cancel(self, mock_post): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertTrue(response.json()["result"]["cancelled"]) + @override_settings(SANDBOX_JWT_PRIVATE_KEY=TEST_RSA_PRIVATE_KEY) + @patch("products.tasks.backend.presentation.views.api.http_requests.post") + def test_command_proxies_side_question(self, mock_post): + reset_sandbox_jwt_key_cache() + self._mock_agent_response( + mock_post, + {"jsonrpc": "2.0", "id": "req-btw", "result": {"answer": "It parses JSONL."}}, + ) + + task = self.create_task() + run = self._create_run_with_sandbox(task) + + response = self.client.post( + self._command_url(task, run), + {"jsonrpc": "2.0", "method": "side_question", "params": {"question": "what does it do?"}, "id": "req-btw"}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json()["result"]["answer"], "It parses JSONL.") + + @parameterized.expand([("blank", " "), ("empty", ""), ("missing", None)]) + def test_command_rejects_side_question_without_a_question(self, _name, question): + task = self.create_task() + run = self._create_run_with_sandbox(task) + params = {} if question is None else {"question": question} + + response = self.client.post( + self._command_url(task, run), + {"jsonrpc": "2.0", "method": "side_question", "params": params, "id": "req-btw"}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_command_side_question_requires_code_access(self): + self.set_tasks_feature_flag(False) + task = self.create_task() + run = self._create_run_with_sandbox(task) + + response = self.client.post( + self._command_url(task, run), + {"jsonrpc": "2.0", "method": "side_question", "params": {"question": "why?"}, "id": "req-btw"}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.json()["code"], "code_access_required") + def test_empty_task_session_returns_read_only_storage_access(self): task = self.create_task(runtime=Task.Runtime.PI) run = self._create_run_with_sandbox(task) diff --git a/products/tasks/frontend/generated/api.schemas.ts b/products/tasks/frontend/generated/api.schemas.ts index 0da815226e2a..5b97a920a539 100644 --- a/products/tasks/frontend/generated/api.schemas.ts +++ b/products/tasks/frontend/generated/api.schemas.ts @@ -3226,6 +3226,7 @@ export const JsonrpcEnumApi = { * * `pi/rpc` - pi/rpc * * `queue_get` - queue_get * * `queue_clear` - queue_clear + * * `side_question` - side_question */ export type TaskRunCommandRequestMethodEnumApi = (typeof TaskRunCommandRequestMethodEnumApi)[keyof typeof TaskRunCommandRequestMethodEnumApi] @@ -3240,6 +3241,7 @@ export const TaskRunCommandRequestMethodEnumApi = { PiRpc: 'pi/rpc', QueueGet: 'queue_get', QueueClear: 'queue_clear', + SideQuestion: 'side_question', } as const /** @@ -3260,7 +3262,8 @@ export interface TaskRunCommandRequestApi { * * `mcp_response` - mcp_response * * `pi/rpc` - pi/rpc * * `queue_get` - queue_get - * * `queue_clear` - queue_clear */ + * * `queue_clear` - queue_clear + * * `side_question` - side_question */ method: TaskRunCommandRequestMethodEnumApi /** Parameters for the command */ params?: TaskRunCommandRequestApiParams diff --git a/products/tasks/frontend/generated/api.ts b/products/tasks/frontend/generated/api.ts index 83b38252e578..5cb1ba472f54 100644 --- a/products/tasks/frontend/generated/api.ts +++ b/products/tasks/frontend/generated/api.ts @@ -1906,7 +1906,7 @@ export const getTasksRunsCommandCreateUrl = (projectId: string, taskId: string, } /** - * Queue user_message JSON-RPC commands through the task workflow and forward sandbox control commands to the agent server. Supports user_message, cancel, close, permission_response, set_config_option, mcp_response, native Pi RPC commands, and Pi queue operations. + * Queue user_message JSON-RPC commands through the task workflow and forward sandbox control commands to the agent server. Supports user_message, cancel, close, permission_response, set_config_option, mcp_response, side_question, native Pi RPC commands, and Pi queue operations. * @summary Send command to task run */ export const tasksRunsCommandCreate = async ( diff --git a/products/tasks/frontend/generated/api.zod.ts b/products/tasks/frontend/generated/api.zod.ts index 06eeb2db49d8..10bd53efb5f1 100644 --- a/products/tasks/frontend/generated/api.zod.ts +++ b/products/tasks/frontend/generated/api.zod.ts @@ -2766,7 +2766,7 @@ export const TasksRunsCancelCreateBody = /* @__PURE__ */ zod.object({ }) /** - * Queue user_message JSON-RPC commands through the task workflow and forward sandbox control commands to the agent server. Supports user_message, cancel, close, permission_response, set_config_option, mcp_response, native Pi RPC commands, and Pi queue operations. + * Queue user_message JSON-RPC commands through the task workflow and forward sandbox control commands to the agent server. Supports user_message, cancel, close, permission_response, set_config_option, mcp_response, side_question, native Pi RPC commands, and Pi queue operations. * @summary Send command to task run */ export const TasksRunsCommandCreateBody = /* @__PURE__ */ zod @@ -2786,12 +2786,13 @@ export const TasksRunsCommandCreateBody = /* @__PURE__ */ zod 'pi/rpc', 'queue_get', 'queue_clear', + 'side_question', ]) .describe( - '\* `user_message` - user_message\n\* `cancel` - cancel\n\* `close` - close\n\* `permission_response` - permission_response\n\* `set_config_option` - set_config_option\n\* `mcp_response` - mcp_response\n\* `pi\/rpc` - pi\/rpc\n\* `queue_get` - queue_get\n\* `queue_clear` - queue_clear' + '\* `user_message` - user_message\n\* `cancel` - cancel\n\* `close` - close\n\* `permission_response` - permission_response\n\* `set_config_option` - set_config_option\n\* `mcp_response` - mcp_response\n\* `pi\/rpc` - pi\/rpc\n\* `queue_get` - queue_get\n\* `queue_clear` - queue_clear\n\* `side_question` - side_question' ) .describe( - 'Command method to execute on the agent server\n\n\* `user_message` - user_message\n\* `cancel` - cancel\n\* `close` - close\n\* `permission_response` - permission_response\n\* `set_config_option` - set_config_option\n\* `mcp_response` - mcp_response\n\* `pi\/rpc` - pi\/rpc\n\* `queue_get` - queue_get\n\* `queue_clear` - queue_clear' + 'Command method to execute on the agent server\n\n\* `user_message` - user_message\n\* `cancel` - cancel\n\* `close` - close\n\* `permission_response` - permission_response\n\* `set_config_option` - set_config_option\n\* `mcp_response` - mcp_response\n\* `pi\/rpc` - pi\/rpc\n\* `queue_get` - queue_get\n\* `queue_clear` - queue_clear\n\* `side_question` - side_question' ), params: zod.record(zod.string(), zod.unknown()).optional().describe('Parameters for the command'), id: zod.unknown().optional().describe('Optional JSON-RPC request ID (string or number)'), diff --git a/services/mcp/src/api/generated.ts b/services/mcp/src/api/generated.ts index 333ecb97338d..930e37bcb920 100644 --- a/services/mcp/src/api/generated.ts +++ b/services/mcp/src/api/generated.ts @@ -77456,6 +77456,7 @@ export namespace Schemas { * * `pi/rpc` - pi/rpc * * `queue_get` - queue_get * * `queue_clear` - queue_clear + * * `side_question` - side_question */ export type TaskRunCommandRequestMethodEnum = typeof TaskRunCommandRequestMethodEnum[keyof typeof TaskRunCommandRequestMethodEnum]; @@ -77470,6 +77471,7 @@ export namespace Schemas { PiRpc: 'pi/rpc', QueueGet: 'queue_get', QueueClear: 'queue_clear', + SideQuestion: 'side_question', } as const; /** @@ -77490,7 +77492,8 @@ export namespace Schemas { * * `mcp_response` - mcp_response * * `pi/rpc` - pi/rpc * * `queue_get` - queue_get - * * `queue_clear` - queue_clear */ + * * `queue_clear` - queue_clear + * * `side_question` - side_question */ method: TaskRunCommandRequestMethodEnum; /** Parameters for the command */ params?: TaskRunCommandRequestParams; From 362632e04fb41fb61a20d783d93eb697a7e0d22a Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Sun, 16 Aug 2026 15:20:02 -0700 Subject: [PATCH 02/15] fix(tasks): return the standard code-access 403 for side_question The gate built its own 403 instead of calling code_access_required_response, so the body omitted the type: permission_denied field every other code-access denial in this viewset carries, and its message said "PostHog Code" where the endpoint's own 403 description says "PostHog Desktop". The proxy test now sends a padded question and asserts the forwarded payload, covering the serializer's strip, and the access test asserts the gate short-circuits before the agent-server call. Claude-Session: https://claude.ai/code/session_01WyZWyiJbQUFSgCfWyM3bCR --- products/tasks/backend/presentation/views/api.py | 13 ++++--------- products/tasks/backend/tests/test_api.py | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/products/tasks/backend/presentation/views/api.py b/products/tasks/backend/presentation/views/api.py index 3942cc4dedf2..d36c40cbb210 100644 --- a/products/tasks/backend/presentation/views/api.py +++ b/products/tasks/backend/presentation/views/api.py @@ -2159,15 +2159,10 @@ def command(self, request, pk=None, **kwargs): request_id = request.validated_data.get("id") params = request.validated_data.get("params") - # Unlike user_message (deliberately ungated below so Inbox "Discuss" works), - # side_question is only surfaced by the Desktop client, so it can require Code access. - if method == "side_question" and not tasks_access.has_tasks_access(request.user): - return Response( - TaskRunErrorResponseSerializer( - {"error": "side_question requires PostHog Code access.", "code": "code_access_required"} - ).data, - status=status.HTTP_403_FORBIDDEN, - ) + # A side question drives the agent and spends model tokens on the caller's behalf. Unlike + # user_message below, it has no Inbox surface to exempt, so every caller takes the gate. + if method == "side_question" and (access_response := code_access_required_response(request.user)): + return access_response if method == "user_message": # The Inbox starts interactive runs and drops the user straight into this composer, diff --git a/products/tasks/backend/tests/test_api.py b/products/tasks/backend/tests/test_api.py index 3bab10615858..f5496bccf050 100644 --- a/products/tasks/backend/tests/test_api.py +++ b/products/tasks/backend/tests/test_api.py @@ -9834,12 +9834,20 @@ def test_command_proxies_side_question(self, mock_post): response = self.client.post( self._command_url(task, run), - {"jsonrpc": "2.0", "method": "side_question", "params": {"question": "what does it do?"}, "id": "req-btw"}, + { + "jsonrpc": "2.0", + "method": "side_question", + "params": {"question": " what does it do? "}, + "id": "req-btw", + }, format="json", ) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.json()["result"]["answer"], "It parses JSONL.") + forwarded = mock_post.call_args[1]["json"] + self.assertEqual(forwarded["method"], "side_question") + self.assertEqual(forwarded["params"]["question"], "what does it do?") @parameterized.expand([("blank", " "), ("empty", ""), ("missing", None)]) def test_command_rejects_side_question_without_a_question(self, _name, question): @@ -9855,7 +9863,8 @@ def test_command_rejects_side_question_without_a_question(self, _name, question) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_command_side_question_requires_code_access(self): + @patch("products.tasks.backend.presentation.views.api.http_requests.post") + def test_command_side_question_requires_code_access(self, mock_post): self.set_tasks_feature_flag(False) task = self.create_task() run = self._create_run_with_sandbox(task) @@ -9868,6 +9877,7 @@ def test_command_side_question_requires_code_access(self): self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertEqual(response.json()["code"], "code_access_required") + mock_post.assert_not_called() def test_empty_task_session_returns_read_only_storage_access(self): task = self.create_task(runtime=Task.Runtime.PI) From 723a2b93e258a157706fa26315f6c5de6e91addb Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Tue, 18 Aug 2026 16:20:13 +0100 Subject: [PATCH 03/15] feat(tasks): bound side_question by usage limit and question length The entitlement gate alone let a team over its usage limit, or in a deactivated org, keep spending model tokens through side_question, while user_message returned 429 in the same situation. The endpoint already documented a 429, so the gate now pairs code_access_required_response with usage_limit_response the way the rest of this viewset does. Question length is the direct lever on what a side question costs, so it takes a 10,000 character cap, following MAX_MCP_RESPONSE_PARAMS_BYTES. The three rejection cases move into test_command_rejects_invalid_payloads, where every other method's validation cases live, and gain an oversized-question row alongside them. Claude-Session: https://claude.ai/code/session_01WyZWyiJbQUFSgCfWyM3bCR --- .../tasks/backend/presentation/serializers.py | 11 +++++- .../tasks/backend/presentation/views/api.py | 9 +++-- products/tasks/backend/tests/test_api.py | 37 ++++++++++++++----- 3 files changed, 44 insertions(+), 13 deletions(-) diff --git a/products/tasks/backend/presentation/serializers.py b/products/tasks/backend/presentation/serializers.py index 68365addfb50..4227865856fa 100644 --- a/products/tasks/backend/presentation/serializers.py +++ b/products/tasks/backend/presentation/serializers.py @@ -2999,6 +2999,10 @@ class TaskRunCommandRequestSerializer(serializers.Serializer): # verbatim and never persisted or captured — they carry data from the user's private systems. MAX_MCP_RESPONSE_PARAMS_BYTES = 300_000 + # A side question is forwarded into an agent turn, so its length is the direct lever on + # model spend. Generous for a one-shot question, far under the request body cap. + MAX_SIDE_QUESTION_CHARS = 10_000 + jsonrpc = serializers.ChoiceField( choices=["2.0"], help_text="JSON-RPC version, must be '2.0'", @@ -3066,7 +3070,12 @@ def validate(self, attrs): ) elif method == "side_question": self._require_nonempty_string(params, "question") - params["question"] = params["question"].strip() + question = params["question"].strip() + if len(question) > self.MAX_SIDE_QUESTION_CHARS: + raise serializers.ValidationError( + {"params": f"question must be at most {self.MAX_SIDE_QUESTION_CHARS} characters"} + ) + params["question"] = question elif method == "pi/rpc": command = params.get("command") if not isinstance(command, dict): diff --git a/products/tasks/backend/presentation/views/api.py b/products/tasks/backend/presentation/views/api.py index d36c40cbb210..e55a381da9cf 100644 --- a/products/tasks/backend/presentation/views/api.py +++ b/products/tasks/backend/presentation/views/api.py @@ -2160,9 +2160,12 @@ def command(self, request, pk=None, **kwargs): params = request.validated_data.get("params") # A side question drives the agent and spends model tokens on the caller's behalf. Unlike - # user_message below, it has no Inbox surface to exempt, so every caller takes the gate. - if method == "side_question" and (access_response := code_access_required_response(request.user)): - return access_response + # user_message below, it has no Inbox surface to exempt, so every caller takes both gates. + if method == "side_question": + if access_response := code_access_required_response(request.user): + return access_response + if limit_response := usage_limit_response(request.user, self.team_id): + return limit_response if method == "user_message": # The Inbox starts interactive runs and drops the user straight into this composer, diff --git a/products/tasks/backend/tests/test_api.py b/products/tasks/backend/tests/test_api.py index f5496bccf050..b19cf0b5f7da 100644 --- a/products/tasks/backend/tests/test_api.py +++ b/products/tasks/backend/tests/test_api.py @@ -9849,23 +9849,26 @@ def test_command_proxies_side_question(self, mock_post): self.assertEqual(forwarded["method"], "side_question") self.assertEqual(forwarded["params"]["question"], "what does it do?") - @parameterized.expand([("blank", " "), ("empty", ""), ("missing", None)]) - def test_command_rejects_side_question_without_a_question(self, _name, question): + @patch("products.tasks.backend.presentation.views.api.http_requests.post") + def test_command_side_question_requires_code_access(self, mock_post): + self.set_tasks_feature_flag(False) task = self.create_task() run = self._create_run_with_sandbox(task) - params = {} if question is None else {"question": question} response = self.client.post( self._command_url(task, run), - {"jsonrpc": "2.0", "method": "side_question", "params": params, "id": "req-btw"}, + {"jsonrpc": "2.0", "method": "side_question", "params": {"question": "why?"}, "id": "req-btw"}, format="json", ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.json()["code"], "code_access_required") + mock_post.assert_not_called() @patch("products.tasks.backend.presentation.views.api.http_requests.post") - def test_command_side_question_requires_code_access(self, mock_post): - self.set_tasks_feature_flag(False) + @patch("products.tasks.backend.logic.services.code_usage_gate.get_posthog_code_usage") + def test_command_side_question_respects_usage_limit(self, mock_gate, mock_post): + mock_gate.return_value = OVER_LIMIT task = self.create_task() run = self._create_run_with_sandbox(task) @@ -9875,8 +9878,8 @@ def test_command_side_question_requires_code_access(self, mock_post): format="json", ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertEqual(response.json()["code"], "code_access_required") + self.assertEqual(response.status_code, status.HTTP_429_TOO_MANY_REQUESTS) + self.assertEqual(response.json()["code"], "usage_limit_exceeded") mock_post.assert_not_called() def test_empty_task_session_returns_read_only_storage_access(self): @@ -10570,6 +10573,22 @@ def test_command_fails_without_sandbox_url(self): }, }, ), + ( + "side_question_blank_question", + {"jsonrpc": "2.0", "method": "side_question", "params": {"question": " "}}, + ), + ( + "side_question_empty_question", + {"jsonrpc": "2.0", "method": "side_question", "params": {"question": ""}}, + ), + ( + "side_question_missing_question", + {"jsonrpc": "2.0", "method": "side_question", "params": {}}, + ), + ( + "side_question_oversized_question", + {"jsonrpc": "2.0", "method": "side_question", "params": {"question": "x" * 10_001}}, + ), ] ) def test_command_rejects_invalid_payloads(self, _name, payload): From 2b1540f3daec787d692432ebdc636c74fb3c9062 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:14 -0700 Subject: [PATCH 04/15] feat: add /btw command for quick side questions Adds a Claude Code-style /btw command: a single-turn, tool-less query forked off the live session transcript that answers without entering the conversation. The exchange renders as an ephemeral card above the composer and leaves no trace in session history, working both mid-turn and while idle. - Adapter answers via a one-shot query() with resume + forkSession, maxTurns 1, an empty toolset, and an always-deny canUseTool, exposed through a new _posthog/side_question extension method. - The sideQuestion capability is advertised in initialize() and plumbed through workspace-server, host-router, and sessionService, gated in the UI by sessionSupportsSideQuestion (local Claude sessions only). Generated-By: PostHog Code Task-Id: c8fbfbf7-10b8-4d4b-8088-c0e9d76075e9 --- .../desktop/apps/web/src/web-host-router.ts | 7 + .../packages/agent/src/acp-extensions.ts | 7 + .../claude/claude-agent.side-question.test.ts | 245 ++++++++++++++++++ .../agent/src/adapters/claude/claude-agent.ts | 120 +++++++++ .../src/adapters/claude/side-question.test.ts | 15 ++ .../src/adapters/claude/side-question.ts | 27 ++ products/desktop/packages/agent/src/index.ts | 6 +- .../core/src/sessions/sessionService.ts | 38 +++ .../sessionServiceSideQuestion.test.ts | 99 +++++++ .../host-router/src/routers/agent.router.ts | 11 + products/desktop/packages/shared/src/index.ts | 1 + .../packages/shared/src/sessions.test.ts | 38 +++ .../desktop/packages/shared/src/sessions.ts | 23 ++ .../features/message-editor/commands.test.ts | 61 ++++- .../src/features/message-editor/commands.ts | 22 ++ .../sessions/components/SessionView.tsx | 2 + .../sessions/components/SideQuestionCard.tsx | 60 +++++ .../sessions/hooks/useSessionCallbacks.ts | 24 ++ .../features/sessions/sideQuestionStore.ts | 63 +++++ .../src/services/agent/agent.ts | 43 +++ .../src/services/agent/schemas.ts | 18 ++ 21 files changed, 927 insertions(+), 3 deletions(-) create mode 100644 products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts create mode 100644 products/desktop/packages/agent/src/adapters/claude/side-question.test.ts create mode 100644 products/desktop/packages/agent/src/adapters/claude/side-question.ts create mode 100644 products/desktop/packages/core/src/sessions/sessionServiceSideQuestion.test.ts create mode 100644 products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx create mode 100644 products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts diff --git a/products/desktop/apps/web/src/web-host-router.ts b/products/desktop/apps/web/src/web-host-router.ts index f531ccc9085c..fc9752f20814 100644 --- a/products/desktop/apps/web/src/web-host-router.ts +++ b/products/desktop/apps/web/src/web-host-router.ts @@ -154,6 +154,13 @@ const agentStubRouter = router({ auth.getState().currentProjectId ?? undefined, ); }), + // Unreachable in practice: the UI gates "/btw" on sessionSupportsSideQuestion, + // which is false for cloud sessions. Present so SessionTrpc stays satisfied. + sideQuestion: publicProcedure + .input(z.object({ sessionId: z.string(), question: z.string() })) + .mutation(() => { + throw new Error("Side questions require a local session"); + }), // Model/mode/effort options for the task-input preview + cloud run creation // (a cloud run requires a model). Real: fetched from the CORS-open PostHog LLM // gateway, same logic the desktop main process runs (see web-agent-config.ts). diff --git a/products/desktop/packages/agent/src/acp-extensions.ts b/products/desktop/packages/agent/src/acp-extensions.ts index 0b4c7c40807f..224a455294c4 100644 --- a/products/desktop/packages/agent/src/acp-extensions.ts +++ b/products/desktop/packages/agent/src/acp-extensions.ts @@ -128,6 +128,13 @@ export const POSTHOG_METHODS = { * completed so the caller can safely send the next prompt. */ REFRESH_SESSION: "_posthog/refresh_session", + + /** + * One-shot side question ("/btw"): forks the live session's transcript into + * a single-turn, tool-less query and returns `{ answer }` without touching + * the main conversation. Payload: `{ question: string }`. + */ + SIDE_QUESTION: "_posthog/side_question", } as const; type PosthogNotification = diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts new file mode 100644 index 000000000000..a5abc64ab6ab --- /dev/null +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts @@ -0,0 +1,245 @@ +import { + type AgentSideConnection, + RequestError, +} from "@agentclientprotocol/sdk"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { POSTHOG_METHODS } from "../../acp-extensions"; +import { Pushable } from "../../utils/streams"; + +type SdkMessage = + | { type: "assistant"; message: { content: unknown[] } } + | { type: "result"; subtype: string }; + +/** Messages the next one-shot query() will yield when iterated. */ +let nextQueryMessages: SdkMessage[] = []; +/** When set, the next query's iterator blocks on this promise before yielding. */ +let nextQueryGate: Promise | null = null; + +const lastQueryCall: { + prompt?: unknown; + options?: Record; +} = {}; + +vi.mock("@anthropic-ai/claude-agent-sdk", () => ({ + query: vi.fn( + (params: { prompt: unknown; options: Record }) => { + lastQueryCall.prompt = params.prompt; + lastQueryCall.options = params.options; + const messages = nextQueryMessages; + const gate = nextQueryGate; + return { + close: vi.fn(), + [Symbol.asyncIterator]: async function* () { + if (gate) await gate; + yield* messages; + }, + }; + }, + ), +})); + +// Import after the mocks so ClaudeAcpAgent resolves the mocked SDK +const { ClaudeAcpAgent } = await import("./claude-agent"); +type Agent = InstanceType; + +function makeAgent(): Agent { + const client = { + sessionUpdate: vi.fn().mockResolvedValue(undefined), + extNotification: vi.fn().mockResolvedValue(undefined), + } as unknown as AgentSideConnection; + return new ClaudeAcpAgent(client); +} + +function installFakeSession( + agent: Agent, + sessionId: string, + overrides: Partial<{ modelId: string }> = {}, +) { + const abortController = new AbortController(); + const input = new Pushable(); + const oldQuery = { close: vi.fn() }; + const hooks = { PreToolUse: [] }; + + const session = { + query: oldQuery, + queryOptions: { + sessionId, + cwd: "/tmp/repo", + model: "claude-sonnet-4-6", + mcpServers: { + posthog: { type: "http", url: "https://example" }, + }, + abortController, + hooks, + }, + input, + modelId: overrides.modelId, + cwd: "/tmp/repo", + } as unknown as Parameters[0]; + + (agent as unknown as { session: unknown }).session = session; + (agent as unknown as { sessionId: string }).sessionId = sessionId; + + return { session, oldQuery, input, abortController }; +} + +function assistantText(text: string): SdkMessage { + return { type: "assistant", message: { content: [{ type: "text", text }] } }; +} + +describe("ClaudeAcpAgent.extMethod side_question", () => { + beforeEach(() => { + nextQueryMessages = []; + nextQueryGate = null; + lastQueryCall.prompt = undefined; + lastQueryCall.options = undefined; + }); + + it.each<[string, Record]>([ + ["missing question", {}], + ["non-string question", { question: 42 }], + ["blank question", { question: " " }], + ])("rejects %s with an invalid-params error", async (_label, params) => { + const agent = makeAgent(); + installFakeSession(agent, "s-1"); + + await expect( + agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, params), + ).rejects.toThrow(/non-empty question/); + }); + + it("forks a one-shot tool-less query and returns the answer", async () => { + const agent = makeAgent(); + const { session, oldQuery, input, abortController } = installFakeSession( + agent, + "s-2", + ); + nextQueryMessages = [ + assistantText("The function "), + assistantText("parses JSONL."), + { type: "result", subtype: "success" }, + ]; + + const result = await agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { + question: "what does it do?", + }); + + expect(result).toEqual({ answer: "The function parses JSONL." }); + + // The prompt is the wrapped question, sent as a self-terminating string. + expect(lastQueryCall.prompt).toContain(""); + expect(lastQueryCall.prompt).toContain("what does it do?"); + + // One-shot fork off the live transcript, toolset removed. + const options = lastQueryCall.options ?? {}; + expect(options.resume).toBe("s-2"); + expect(options.forkSession).toBe(true); + expect(options.maxTurns).toBe(1); + expect(options.tools).toEqual([]); + expect(options.allowedTools).toEqual([]); + expect(options.mcpServers).toEqual({}); + expect(options.sessionId).toBeUndefined(); + expect(options.hooks).toBeUndefined(); + // Fresh controller: aborting the one-shot must not poison the session. + expect(options.abortController).not.toBe(abortController); + + // canUseTool hard-denies anything that slips past the empty toolset. + const canUseTool = options.canUseTool as () => Promise<{ + behavior: string; + }>; + await expect(canUseTool()).resolves.toMatchObject({ behavior: "deny" }); + + // The live session is untouched. + const live = session as { + query: unknown; + input: unknown; + queryOptions: { sessionId: string }; + }; + expect(live.query).toBe(oldQuery); + expect(live.input).toBe(input); + expect(live.queryOptions.sessionId).toBe("s-2"); + expect((agent as unknown as { sessionId: string }).sessionId).toBe("s-2"); + }); + + it("answers on the live session model, not the creation-time option", async () => { + const agent = makeAgent(); + installFakeSession(agent, "s-3", { modelId: "claude-opus-4-8" }); + nextQueryMessages = [ + assistantText("yes"), + { type: "result", subtype: "success" }, + ]; + + await agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "hm?" }); + + expect(lastQueryCall.options?.model).toContain("opus"); + }); + + it("surfaces an error result subtype as a RequestError", async () => { + const agent = makeAgent(); + installFakeSession(agent, "s-4"); + nextQueryMessages = [{ type: "result", subtype: "error_max_turns" }]; + + const promise = agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { + question: "hm?", + }); + await expect(promise).rejects.toBeInstanceOf(RequestError); + await expect(promise).rejects.toThrow(/error_max_turns/); + }); + + it("rejects when the fork produces no answer text", async () => { + const agent = makeAgent(); + installFakeSession(agent, "s-5"); + nextQueryMessages = [{ type: "result", subtype: "success" }]; + + await expect( + agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "hm?" }), + ).rejects.toThrow(/no answer/); + }); + + it("allows only one side question at a time", async () => { + const agent = makeAgent(); + installFakeSession(agent, "s-6"); + + let release: () => void = () => {}; + nextQueryGate = new Promise((resolve) => { + release = resolve; + }); + nextQueryMessages = [ + assistantText("first"), + { type: "result", subtype: "success" }, + ]; + + const first = agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { + question: "first?", + }); + await expect( + agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "second?" }), + ).rejects.toThrow(/already in progress/); + + release(); + await expect(first).resolves.toEqual({ answer: "first" }); + + // The guard clears once the first completes. + nextQueryGate = null; + nextQueryMessages = [ + assistantText("third"), + { type: "result", subtype: "success" }, + ]; + await expect( + agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "third?" }), + ).resolves.toEqual({ answer: "third" }); + }); + + it("wraps SDK failures (e.g. nothing to resume) in a clear RequestError", async () => { + const agent = makeAgent(); + installFakeSession(agent, "s-7"); + nextQueryGate = Promise.reject(new Error("No conversation found")); + nextQueryGate.catch(() => {}); + + const promise = agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { + question: "too early?", + }); + await expect(promise).rejects.toBeInstanceOf(RequestError); + await expect(promise).rejects.toThrow(/No conversation found/); + }); +}); diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts index 743a5ce84e98..37809ea4ee35 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts @@ -146,6 +146,10 @@ import { toSdkEffort, } from "./session/options"; import { SettingsManager } from "./session/settings"; +import { + buildSideQuestionPrompt, + SIDE_QUESTION_TIMEOUT_MS, +} from "./side-question"; import { CODE_EXECUTION_MODES, type CodeExecutionMode, @@ -351,6 +355,8 @@ export class ClaudeAcpAgent extends BaseAcpAgent { private options?: ClaudeAcpAgentOptions; private enrichment?: Enrichment; private enrichedReadCache: EnrichedReadCache = new Map(); + /** One side question at a time; bounds concurrent forks off the transcript. */ + private sideQuestionInFlight = false; constructor(client: AgentSideConnection, options?: ClaudeAcpAgentOptions) { super(client); @@ -409,6 +415,7 @@ export class ClaudeAcpAgent extends BaseAcpAgent { // retire, so the clear has to look unavailable rather than silently // not take. conversationClear: true, + sideQuestion: true, }, claudeCode: { promptQueueing: true, @@ -1626,6 +1633,16 @@ export class ClaudeAcpAgent extends BaseAcpAgent { method: string, params: Record, ): Promise> { + if (isMethod(method, POSTHOG_METHODS.SIDE_QUESTION)) { + if (typeof params.question !== "string" || !params.question.trim()) { + throw new RequestError( + -32602, + "side_question requires a non-empty question", + ); + } + return await this.answerSideQuestion(params.question); + } + if (!isMethod(method, POSTHOG_METHODS.REFRESH_SESSION)) { throw RequestError.methodNotFound(method); } @@ -1948,6 +1965,109 @@ export class ClaudeAcpAgent extends BaseAcpAgent { return { stopReason: "end_turn" }; } + /** + * Answers a "/btw" side question by forking the live session's transcript + * into a one-shot, tool-less, single-turn query. Strictly non-mutating: + * the fork gets its own SDK session id and AbortController, and nothing on + * `this.session` is touched, so the main conversation (including an + * in-flight turn) never sees the exchange. + */ + private async answerSideQuestion( + question: string, + ): Promise<{ answer: string }> { + if (this.sideQuestionInFlight) { + throw new RequestError(-32002, "A side question is already in progress"); + } + this.sideQuestionInFlight = true; + + const abortController = new AbortController(); + try { + // Drop `sessionId` (identity comes from `resume`) and `hooks` (they + // close over live-session caches and task state). + const { + sessionId: _drop, + hooks: _hooks, + ...rest + } = this.session.queryOptions; + const options: Options = { + ...rest, + resume: this.sessionId, + forkSession: true, + maxTurns: 1, + // Belt and braces: remove the toolset entirely and deny anything + // that slips through; the prompt wrapper also says "no tools". + tools: [], + allowedTools: [], + canUseTool: async () => ({ + behavior: "deny", + message: "Tools are unavailable while answering a side question.", + interrupt: false, + }), + // Never reuse in-process MCP instances ("Already connected to a + // transport"); the fork has no tools, so it needs no servers. + mcpServers: {}, + abortController, + // `rest.model` is the creation-time value; the user may have + // switched models since, so answer on the live session model. + ...(this.session.modelId && { + model: toSdkModelId(this.session.modelId), + }), + }; + + const oneShot = query({ + prompt: buildSideQuestionPrompt(question), + options, + }); + + const answer = await withTimeout( + (async () => { + const chunks: string[] = []; + for await (const message of oneShot) { + if (message.type === "assistant") { + for (const block of message.message.content) { + if (block.type === "text") { + chunks.push(block.text); + } + } + } else if (message.type === "result") { + if (message.subtype !== "success") { + throw new RequestError( + -32603, + `Side question failed: ${message.subtype}`, + ); + } + break; + } + } + return chunks.join("").trim(); + })(), + SIDE_QUESTION_TIMEOUT_MS, + ); + + if (answer.result === "timeout") { + throw new RequestError( + -32603, + `Side question timed out after ${SIDE_QUESTION_TIMEOUT_MS}ms`, + ); + } + if (!answer.value) { + throw new RequestError(-32603, "Side question produced no answer"); + } + return { answer: answer.value }; + } catch (error) { + if (error instanceof RequestError) throw error; + // A brand-new session has no transcript on disk yet, so `resume` has + // nothing to fork; surface that case clearly. + throw new RequestError( + -32603, + `Side question failed: ${error instanceof Error ? error.message : String(error)}`, + ); + } finally { + abortController.abort(); + this.sideQuestionInFlight = false; + } + } + private async refreshSession( mcpServers: Record, ): Promise { diff --git a/products/desktop/packages/agent/src/adapters/claude/side-question.test.ts b/products/desktop/packages/agent/src/adapters/claude/side-question.test.ts new file mode 100644 index 000000000000..73f375a302d7 --- /dev/null +++ b/products/desktop/packages/agent/src/adapters/claude/side-question.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it } from "vitest"; +import { buildSideQuestionPrompt } from "./side-question"; + +describe("buildSideQuestionPrompt", () => { + it("wraps the question in the side-question constraints", () => { + const prompt = buildSideQuestionPrompt("what does this function do?"); + + expect(prompt).toMatch( + /^[\s\S]*<\/system-reminder>\n\nwhat does this function do\?$/, + ); + expect(prompt).toContain("no tools available"); + expect(prompt).toContain("one-off response"); + expect(prompt).toContain("Never promise actions"); + }); +}); diff --git a/products/desktop/packages/agent/src/adapters/claude/side-question.ts b/products/desktop/packages/agent/src/adapters/claude/side-question.ts new file mode 100644 index 000000000000..be6917625bd2 --- /dev/null +++ b/products/desktop/packages/agent/src/adapters/claude/side-question.ts @@ -0,0 +1,27 @@ +/** + * Helpers for the one-shot "/btw" side question. Pure functions only — + * the forked query mechanics live in ClaudeAcpAgent.answerSideQuestion. + */ + +export const SIDE_QUESTION_TIMEOUT_MS = 120_000; + +/** + * Wraps the user's question in the constraints the fork must obey: no tools, + * single one-off response, answer only from what the conversation already + * contains. Mirrors the prompt shape Claude Code uses for its /btw command. + */ +export function buildSideQuestionPrompt(question: string): string { + return [ + "", + "The user is asking a quick side question. This is a one-off response", + "outside the main conversation; nothing you say here will be visible in", + "later turns. You have no tools available: you cannot read files, run", + "commands, search, or take any action. Answer concisely using only the", + "knowledge already present in the conversation. Never promise actions", + '(do not say things like "Let me try..." or offer to investigate);', + "if you do not know the answer, say so plainly.", + "", + "", + question, + ].join("\n"); +} diff --git a/products/desktop/packages/agent/src/index.ts b/products/desktop/packages/agent/src/index.ts index dd4e6081783c..4acb307033b7 100644 --- a/products/desktop/packages/agent/src/index.ts +++ b/products/desktop/packages/agent/src/index.ts @@ -1,4 +1,8 @@ -export { isNotification, POSTHOG_NOTIFICATIONS } from "./acp-extensions"; +export { + isNotification, + POSTHOG_METHODS, + POSTHOG_NOTIFICATIONS, +} from "./acp-extensions"; export { getMcpToolMetadata, isMcpToolReadOnly, diff --git a/products/desktop/packages/core/src/sessions/sessionService.ts b/products/desktop/packages/core/src/sessions/sessionService.ts index ae494c8fead3..60acb97c7190 100644 --- a/products/desktop/packages/core/src/sessions/sessionService.ts +++ b/products/desktop/packages/core/src/sessions/sessionService.ts @@ -47,6 +47,7 @@ import { type StoredLogEntry, sendableQueuePrefixLength, sessionSupportsNativeSteer, + sessionSupportsSideQuestion, type TaskRunArtifact, type TaskRunStatus, TRANSCRIPT_TAIL_WINDOW, @@ -316,6 +317,7 @@ export interface SessionTrpc { reconnect: TrpcMutation; cancel: TrpcMutation; prompt: TrpcMutation; + sideQuestion: TrpcMutation; cancelPrompt: TrpcMutation; cancelPermission: TrpcMutation; respondToPermission: TrpcMutation; @@ -1676,6 +1678,11 @@ function readSteering(result: unknown): string | undefined { return (result as { steering?: string } | undefined)?.steering; } +/** The side-question capability on a loosely-typed agent start/reconnect result. */ +function readSideQuestion(result: unknown): boolean | undefined { + return (result as { sideQuestion?: boolean } | undefined)?.sideQuestion; +} + // Live streaming emits agent_message_chunk; SessionLogWriter coalesces a chunk // run into a single agent_message, so hydration and cloud replay read the same // text back as that final. Both forms carry { type: "text", text }, so both @@ -2328,6 +2335,7 @@ export class SessionService { status: "connected", configOptions, steering: readSteering(result), + sideQuestion: readSideQuestion(result), }); // Persist the merged config options @@ -2685,6 +2693,7 @@ export class SessionService { | undefined; session.configOptions = configOptions; session.steering = readSteering(result); + session.sideQuestion = readSideQuestion(result); // Persist the config options if (configOptions) { @@ -4250,6 +4259,35 @@ export class SessionService { }); } + /** + * Ask a one-shot "/btw" side question: a single-turn, tool-less query forked + * off the live transcript. The exchange never enters the conversation, so + * this bypasses the queue/steer machinery entirely and is valid both + * mid-turn and idle. + */ + async askSideQuestion(taskId: string, question: string): Promise { + if (!this.d.getIsOnline()) { + throw new Error( + "No internet connection. Please check your connection and try again.", + ); + } + + const session = this.d.store.getSessionByTaskId(taskId); + if (!session) throw new Error("No active session for task"); + if (!sessionSupportsSideQuestion(session)) { + throw new Error("Side questions aren't supported for this session yet."); + } + if (session.status !== "connected") { + throw new Error("Session is not ready. Try again once it's connected."); + } + + const result = (await this.d.trpc.agent.sideQuestion.mutate({ + sessionId: session.taskRunId, + question, + })) as { answer: string }; + return result.answer; + } + /** * Send the next queued message as its own prompt. * Called internally when a turn completes and there are queued messages. diff --git a/products/desktop/packages/core/src/sessions/sessionServiceSideQuestion.test.ts b/products/desktop/packages/core/src/sessions/sessionServiceSideQuestion.test.ts new file mode 100644 index 000000000000..8c1ffbf5abb6 --- /dev/null +++ b/products/desktop/packages/core/src/sessions/sessionServiceSideQuestion.test.ts @@ -0,0 +1,99 @@ +import type { AgentSession } from "@posthog/shared"; +import { describe, expect, it, vi } from "vitest"; +import { SessionService, type SessionServiceDeps } from "./sessionService"; + +function makeSession(overrides: Partial = {}): AgentSession { + return { + taskRunId: "run-1", + taskId: "task-1", + taskTitle: "Test task", + channel: "", + events: [], + startedAt: 1, + status: "connected", + isPromptPending: false, + isCompacting: false, + promptStartedAt: null, + pendingPermissions: new Map(), + pausedDurationMs: 0, + messageQueue: [], + optimisticItems: [], + adapter: "claude", + ...overrides, + } as AgentSession; +} + +function createHarness(session: AgentSession | undefined, online = true) { + const sideQuestionMutate = vi.fn().mockResolvedValue({ answer: "42" }); + const deps = { + store: { + getSessionByTaskId: () => session, + }, + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + getIsOnline: () => online, + trpc: { + agent: { + sideQuestion: { mutate: sideQuestionMutate }, + onSessionIdleKilled: { + subscribe: () => ({ unsubscribe: vi.fn() }), + }, + }, + }, + } as unknown as SessionServiceDeps; + + return { service: new SessionService(deps), sideQuestionMutate }; +} + +describe("SessionService.askSideQuestion", () => { + it("sends the question keyed by taskRunId and returns the answer", async () => { + const { service, sideQuestionMutate } = createHarness(makeSession()); + + await expect(service.askSideQuestion("task-1", "why?")).resolves.toBe("42"); + expect(sideQuestionMutate).toHaveBeenCalledWith({ + sessionId: "run-1", + question: "why?", + }); + }); + + it("rejects while offline", async () => { + const { service, sideQuestionMutate } = createHarness(makeSession(), false); + + await expect(service.askSideQuestion("task-1", "why?")).rejects.toThrow( + /No internet connection/, + ); + expect(sideQuestionMutate).not.toHaveBeenCalled(); + }); + + it("rejects when no session exists for the task", async () => { + const { service } = createHarness(undefined); + + await expect(service.askSideQuestion("task-1", "why?")).rejects.toThrow( + /No active session/, + ); + }); + + it.each<[string, Partial]>([ + ["cloud session", { isCloud: true, sideQuestion: true }], + ["codex without the capability", { adapter: "codex" }], + ])("rejects a %s as unsupported", async (_label, overrides) => { + const { service, sideQuestionMutate } = createHarness( + makeSession(overrides), + ); + + await expect(service.askSideQuestion("task-1", "why?")).rejects.toThrow( + /aren't supported/, + ); + expect(sideQuestionMutate).not.toHaveBeenCalled(); + }); + + it("rejects when the session is not connected", async () => { + const { service, sideQuestionMutate } = createHarness( + makeSession({ status: "connecting" }), + ); + + await expect(service.askSideQuestion("task-1", "why?")).rejects.toThrow( + /not ready/, + ); + expect(sideQuestionMutate).not.toHaveBeenCalled(); + }); +}); diff --git a/products/desktop/packages/host-router/src/routers/agent.router.ts b/products/desktop/packages/host-router/src/routers/agent.router.ts index 4c85c6158c3d..9d8a43f16432 100644 --- a/products/desktop/packages/host-router/src/routers/agent.router.ts +++ b/products/desktop/packages/host-router/src/routers/agent.router.ts @@ -24,6 +24,8 @@ import { rtkStatusOutput, sessionResponseSchema, setConfigOptionInput, + sideQuestionInput, + sideQuestionOutput, startSessionInput, subscribeSessionInput, } from "@posthog/workspace-server/services/agent/schemas"; @@ -51,6 +53,15 @@ export const agentRouter = router({ }), ), + sideQuestion: publicProcedure + .input(sideQuestionInput) + .output(sideQuestionOutput) + .mutation(({ ctx, input }) => + ctx.container + .get(AGENT_SERVICE) + .sideQuestion(input.sessionId, input.question), + ), + cancel: publicProcedure .input(cancelSessionInput) .mutation(({ ctx, input }) => diff --git a/products/desktop/packages/shared/src/index.ts b/products/desktop/packages/shared/src/index.ts index 177679dcb4c0..38b827c73566 100644 --- a/products/desktop/packages/shared/src/index.ts +++ b/products/desktop/packages/shared/src/index.ts @@ -326,6 +326,7 @@ export { type SessionStatus, sendableQueuePrefixLength, sessionSupportsNativeSteer, + sessionSupportsSideQuestion, TRANSCRIPT_TAIL_WINDOW, } from "./sessions"; export type { diff --git a/products/desktop/packages/shared/src/sessions.test.ts b/products/desktop/packages/shared/src/sessions.test.ts index 8706578afe1e..04fc7a35d38a 100644 --- a/products/desktop/packages/shared/src/sessions.test.ts +++ b/products/desktop/packages/shared/src/sessions.test.ts @@ -4,6 +4,7 @@ import { type AgentSession, resolveBypassRevertMode, sessionSupportsNativeSteer, + sessionSupportsSideQuestion, } from "./sessions"; function modeOption( @@ -115,3 +116,40 @@ describe("sessionSupportsNativeSteer", () => { expect(sessionSupportsNativeSteer(session)).toBe(expected); }); }); + +describe("sessionSupportsSideQuestion", () => { + type Case = Pick; + + it.each<[string, Case, boolean]>([ + [ + "claude advertises the capability", + { isCloud: false, sideQuestion: true, adapter: "claude" }, + true, + ], + // Fallback: pre-capability start paths leave sideQuestion unset; local + // claude is assumed capable, matching the steer fallback. + [ + "claude with no capability (fallback)", + { isCloud: false, sideQuestion: undefined, adapter: "claude" }, + true, + ], + [ + "codex with no capability (no fallback)", + { isCloud: false, sideQuestion: undefined, adapter: "codex" }, + false, + ], + [ + "claude explicitly without the capability", + { isCloud: false, sideQuestion: false, adapter: "claude" }, + false, + ], + // The fork runs against the local transcript, so cloud never qualifies. + [ + "cloud claude with the capability", + { isCloud: true, sideQuestion: true, adapter: "claude" }, + false, + ], + ])("%s", (_label, session, expected) => { + expect(sessionSupportsSideQuestion(session)).toBe(expected); + }); +}); diff --git a/products/desktop/packages/shared/src/sessions.ts b/products/desktop/packages/shared/src/sessions.ts index 2626e0202e16..a0f3dc8fcc1f 100644 --- a/products/desktop/packages/shared/src/sessions.ts +++ b/products/desktop/packages/shared/src/sessions.ts @@ -96,6 +96,12 @@ export interface AgentSession { * record a conversation-cleared boundary they would ignore on resume. */ conversationClear?: boolean; + /** + * Adapter's negotiated side-question capability (`_meta.posthog.sideQuestion` + * from initialize). True means the adapter can answer a one-shot "/btw" + * question forked off the live transcript without touching the conversation. + */ + sideQuestion?: boolean; pendingPermissions: Map; pausedDurationMs: number; messageQueue: QueuedMessage[]; @@ -278,3 +284,20 @@ export function sessionSupportsNativeSteer( if (session.isCloud) return false; return session.steering == null && session.adapter === "claude"; } + +/** + * Whether the session can answer a one-shot "/btw" side question (a + * single-turn, tool-less query forked off the live transcript). Decided by the + * adapter's negotiated `sideQuestion` capability; cloud sessions can't — the + * fork runs against the local transcript. + * + * Fallback: if `sideQuestion` is unset (a session started before capability + * plumbing), local Claude is assumed capable, matching the steer fallback. + */ +export function sessionSupportsSideQuestion( + session: Pick, +): boolean { + if (session.isCloud) return false; + if (session.sideQuestion === true) return true; + return session.sideQuestion == null && session.adapter === "claude"; +} diff --git a/products/desktop/packages/ui/src/features/message-editor/commands.test.ts b/products/desktop/packages/ui/src/features/message-editor/commands.test.ts index 420de06c2f79..18cc6edd58c7 100644 --- a/products/desktop/packages/ui/src/features/message-editor/commands.test.ts +++ b/products/desktop/packages/ui/src/features/message-editor/commands.test.ts @@ -1,7 +1,15 @@ -import { describe, expect, it } from "vitest"; -import { rewriteLocalSkillCommandPrompt } from "./commands"; +import { describe, expect, it, vi } from "vitest"; +import { + rewriteLocalSkillCommandPrompt, + tryExecuteCodeCommand, +} from "./commands"; import type { EditorAvailableCommand } from "./types"; +const toastError = vi.hoisted(() => vi.fn()); +vi.mock("@posthog/ui/primitives/toast", () => ({ + toast: { error: toastError, success: vi.fn() }, +})); + const commands: EditorAvailableCommand[] = [ { name: "local-test-skill", @@ -38,3 +46,52 @@ describe("message editor commands", () => { ).toBe(null); }); }); + +describe("/btw command", () => { + const baseContext = { + taskId: "task-1", + repoPath: "/tmp/repo", + session: null, + taskRun: null, + }; + + it("routes the question to askSideQuestion and reports handled", async () => { + const askSideQuestion = vi.fn(); + const handled = await tryExecuteCodeCommand("/btw what changed here?", { + ...baseContext, + askSideQuestion, + }); + expect(handled).toBe(true); + expect(askSideQuestion).toHaveBeenCalledWith("what changed here?"); + }); + + it("toasts on an empty question without calling askSideQuestion", async () => { + toastError.mockClear(); + const askSideQuestion = vi.fn(); + const handled = await tryExecuteCodeCommand("/btw", { + ...baseContext, + askSideQuestion, + }); + expect(handled).toBe(true); + expect(askSideQuestion).not.toHaveBeenCalled(); + expect(toastError).toHaveBeenCalledWith("Add a question after /btw"); + }); + + it("toasts when the session doesn't support side questions", async () => { + toastError.mockClear(); + const handled = await tryExecuteCodeCommand("/btw is this supported?", { + ...baseContext, + askSideQuestion: undefined, + }); + expect(handled).toBe(true); + expect(toastError).toHaveBeenCalledWith( + "Side questions aren't supported for this session yet.", + ); + }); + + it("leaves non-command text unhandled", async () => { + expect(await tryExecuteCodeCommand("btw not a command", baseContext)).toBe( + false, + ); + }); +}); diff --git a/products/desktop/packages/ui/src/features/message-editor/commands.ts b/products/desktop/packages/ui/src/features/message-editor/commands.ts index 8fb6956755a6..531942ed823e 100644 --- a/products/desktop/packages/ui/src/features/message-editor/commands.ts +++ b/products/desktop/packages/ui/src/features/message-editor/commands.ts @@ -26,6 +26,8 @@ interface CommandContext { events: unknown[]; } | null; taskRun: { id?: string; log_url?: string } | null; + /** Fires a "/btw" side question. Absent when the session doesn't support them. */ + askSideQuestion?: (question: string) => void; } export interface CodeCommandInsertContext { @@ -104,8 +106,28 @@ const addDirCommand: CodeCommand = { }, }; +const btwCommand: CodeCommand = { + name: "btw", + description: + "Ask a quick side question without interrupting the conversation", + input: { hint: "your question" }, + execute(args, ctx) { + const question = args?.trim(); + if (!question) { + toast.error("Add a question after /btw"); + return; + } + if (!ctx.askSideQuestion) { + toast.error("Side questions aren't supported for this session yet."); + return; + } + ctx.askSideQuestion(question); + }, +}; + const commands: CodeCommand[] = [ addDirCommand, + btwCommand, makeFeedbackCommand("good", "good", "Positive"), makeFeedbackCommand("bad", "bad", "Negative"), makeFeedbackCommand("feedback", "general", "General"), diff --git a/products/desktop/packages/ui/src/features/sessions/components/SessionView.tsx b/products/desktop/packages/ui/src/features/sessions/components/SessionView.tsx index ac988c273e36..f224d22339bc 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/SessionView.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/SessionView.tsx @@ -42,6 +42,7 @@ import { QueuedMessagesDock } from "@posthog/ui/features/sessions/components/Que import { ReasoningLevelSelector } from "@posthog/ui/features/sessions/components/ReasoningLevelSelector"; import { RawLogsView } from "@posthog/ui/features/sessions/components/raw-logs/RawLogsView"; import { SessionInitializingView } from "@posthog/ui/features/sessions/components/SessionInitializingView"; +import { SideQuestionCard } from "@posthog/ui/features/sessions/components/SideQuestionCard"; import { SteerQueueToggle } from "@posthog/ui/features/sessions/components/SteerQueueToggle"; import { isSubmittedContentUnchanged, @@ -740,6 +741,7 @@ export function SessionView({ }`} > + {taskId && } {taskId && } s.byTaskId[taskId]); + const dismiss = useSideQuestionStore((s) => s.dismiss); + + if (!entry) return null; + + return ( + + + + + {entry.question} + + + dismiss(taskId)} + > + + + + + + {entry.status === "pending" && ( + + + Answering… + + )} + {entry.status === "done" && ( + + + + )} + {entry.status === "error" && ( + + {entry.error ?? "Side question failed"} + + )} + + + ); +} diff --git a/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts b/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts index f4b29a3e90ba..655798177d98 100644 --- a/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts +++ b/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts @@ -13,6 +13,7 @@ import { } from "@posthog/core/sessions/sessionService"; import { useService } from "@posthog/di/react"; import { useHostTRPCClient } from "@posthog/host-router/react"; +import { sessionSupportsSideQuestion } from "@posthog/shared"; import type { Task } from "@posthog/shared/domain-types"; import { resolveLocalSkillPrompt, @@ -25,6 +26,7 @@ import { type AgentSession, sessionStoreSetters, } from "@posthog/ui/features/sessions/sessionStore"; +import { useSideQuestionStore } from "@posthog/ui/features/sessions/sideQuestionStore"; import { useTaskViewed } from "@posthog/ui/features/sidebar/useTaskViewed"; import { SHELL_CLIENT, @@ -76,6 +78,28 @@ export function useSessionCallbacks({ } : null, taskRun: task.latest_run ?? null, + // Fire-and-forget so the composer clears immediately; the side + // question runs beside the conversation, mid-turn or idle. + askSideQuestion: + currentSession && sessionSupportsSideQuestion(currentSession) + ? (question) => { + const { ask, resolve, fail } = useSideQuestionStore.getState(); + const id = ask(taskId, question); + sessionService + .askSideQuestion(taskId, question) + .then((answer) => resolve(taskId, id, answer)) + .catch((error) => { + fail( + taskId, + id, + error instanceof Error + ? error.message + : "Side question failed", + ); + log.error("Side question failed", error); + }); + } + : undefined, }); if (handled) return true; diff --git a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts new file mode 100644 index 000000000000..47f7e6b81480 --- /dev/null +++ b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts @@ -0,0 +1,63 @@ +import { create } from "zustand"; + +export type SideQuestionStatus = "pending" | "done" | "error"; + +export interface SideQuestionEntry { + id: string; + question: string; + answer: string; + status: SideQuestionStatus; + error?: string; +} + +interface SideQuestionState { + /** Latest side question per task. Ephemeral: never persisted, never part of session history. */ + byTaskId: Record; + ask: (taskId: string, question: string) => string; + resolve: (taskId: string, id: string, answer: string) => void; + fail: (taskId: string, id: string, error: string) => void; + dismiss: (taskId: string) => void; +} + +let nextId = 0; + +export const useSideQuestionStore = create()((set) => ({ + byTaskId: {}, + ask: (taskId, question) => { + const id = `sq-${++nextId}`; + set((state) => ({ + byTaskId: { + ...state.byTaskId, + [taskId]: { id, question, answer: "", status: "pending" }, + }, + })); + return id; + }, + resolve: (taskId, id, answer) => + set((state) => { + const entry = state.byTaskId[taskId]; + if (entry?.id !== id) return state; + return { + byTaskId: { + ...state.byTaskId, + [taskId]: { ...entry, answer, status: "done" }, + }, + }; + }), + fail: (taskId, id, error) => + set((state) => { + const entry = state.byTaskId[taskId]; + if (entry?.id !== id) return state; + return { + byTaskId: { + ...state.byTaskId, + [taskId]: { ...entry, error, status: "error" }, + }, + }; + }), + dismiss: (taskId) => + set((state) => { + const { [taskId]: _dismissed, ...rest } = state.byTaskId; + return { byTaskId: rest }; + }), +})); diff --git a/products/desktop/packages/workspace-server/src/services/agent/agent.ts b/products/desktop/packages/workspace-server/src/services/agent/agent.ts index 8c29492cbd2e..4f659599f42e 100644 --- a/products/desktop/packages/workspace-server/src/services/agent/agent.ts +++ b/products/desktop/packages/workspace-server/src/services/agent/agent.ts @@ -16,6 +16,7 @@ import { detectRtkBinary, isMcpToolReadOnly, isNotification, + POSTHOG_METHODS, POSTHOG_NOTIFICATIONS, } from "@posthog/agent"; import type { McpToolApprovals } from "@posthog/agent/adapters/claude/mcp/tool-metadata"; @@ -112,7 +113,9 @@ import { type ReconnectSessionInput, type RtkStatus, type SessionResponse, + type SideQuestionOutput, type StartSessionInput, + sideQuestionOutput, } from "./schemas"; export type { InterruptReason }; @@ -314,6 +317,16 @@ function extractSteeringCapability(init: unknown): string | undefined { /** A streaming turn emits many events a second; warn once a minute, not per event. */ const NO_LISTENER_WARN_INTERVAL_MS = 60_000; +/** Pull the adapter's `agentCapabilities._meta.posthog.sideQuestion` from initialize. */ +function extractSideQuestionCapability(init: unknown): boolean | undefined { + const sideQuestion = ( + init as { + agentCapabilities?: { _meta?: { posthog?: { sideQuestion?: unknown } } }; + } + )?.agentCapabilities?._meta?.posthog?.sideQuestion; + return typeof sideQuestion === "boolean" ? sideQuestion : undefined; +} + interface ManagedSession { taskRunId: string; taskId: string; @@ -330,6 +343,8 @@ interface ManagedSession { configOptions?: SessionConfigOption[]; /** Adapter's negotiated steering capability from initialize (`_meta.posthog.steering`). */ steering?: string; + /** Adapter's negotiated side-question capability from initialize (`_meta.posthog.sideQuestion`). */ + sideQuestion?: boolean; /** Tracks in-flight MCP tool calls (toolCallId → toolKey) for cancellation */ inFlightMcpToolCalls: Map; /** MCP tool approval states fetched at session start */ @@ -945,6 +960,7 @@ If a repository is required, call \`list_repos\` to find it, then use \`clone_re // the host gates steer-vs-resend on the negotiated capability, not on a // hardcoded adapter name (codex-acp advertises "interrupt-resend"). const steering = extractSteeringCapability(initResult); + const sideQuestion = extractSideQuestionCapability(initResult); const { servers: mcpServers, @@ -1191,6 +1207,7 @@ If a repository is required, call \`list_repos\` to find it, then use \`clone_re promptPending: false, configOptions, steering, + sideQuestion, inFlightMcpToolCalls: new Map(), mcpToolApprovals: toolApprovals, toolInstallations, @@ -1457,6 +1474,31 @@ If a repository is required, call \`list_repos\` to find it, then use \`clone_re } } + /** + * Answers a one-shot "/btw" side question via the adapter's SIDE_QUESTION + * extension method. Deliberately does not touch promptPending or the + * sleep/idle lifecycle: the exchange runs beside the conversation (ACP + * JSON-RPC multiplexes, so this works mid-turn) and never becomes part of it. + */ + async sideQuestion( + sessionId: string, + question: string, + ): Promise { + const session = this.sessions.get(sessionId); + if (!session) { + throw new Error(`Session not found: ${sessionId}`); + } + + session.lastActivityAt = Date.now(); + this.recordActivity(sessionId); + + const result = await session.clientSideConnection.extMethod( + POSTHOG_METHODS.SIDE_QUESTION, + { sessionId: getAgentSessionId(session), question }, + ); + return sideQuestionOutput.parse(result); + } + async cancelSession(sessionId: string): Promise { const session = this.sessions.get(sessionId); if (!session) return false; @@ -2218,6 +2260,7 @@ For git operations while detached: channel: session.channel, configOptions: session.configOptions, steering: session.steering, + sideQuestion: session.sideQuestion, }; } diff --git a/products/desktop/packages/workspace-server/src/services/agent/schemas.ts b/products/desktop/packages/workspace-server/src/services/agent/schemas.ts index 14d1cd3ba438..230227a592a1 100644 --- a/products/desktop/packages/workspace-server/src/services/agent/schemas.ts +++ b/products/desktop/packages/workspace-server/src/services/agent/schemas.ts @@ -178,6 +178,10 @@ export const sessionResponseSchema = z.object({ // running turn; "interrupt-resend" (legacy) or absent means the host must // cancel + resend instead. Drives the host's steer-vs-resend decision. steering: z.string().optional(), + // The adapter's negotiated side-question capability from initialize + // (`_meta.posthog.sideQuestion`): true means the adapter can answer a + // one-shot "/btw" question forked off the live transcript. + sideQuestion: z.boolean().optional(), }); export type SessionResponse = z.infer; @@ -208,6 +212,20 @@ export const promptOutput = z.object({ export type PromptOutput = z.infer; +// Side question ("/btw") input/output +export const sideQuestionInput = z.object({ + sessionId: z.string(), + question: z.string().min(1), +}); + +export type SideQuestionInput = z.infer; + +export const sideQuestionOutput = z.object({ + answer: z.string(), +}); + +export type SideQuestionOutput = z.infer; + // Cancel session input export const cancelSessionInput = z.object({ sessionId: z.string(), From 18c5dc2d5797f4e716f72d57279845b2c5cb9ec5 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:16 -0700 Subject: [PATCH 05/15] refactor: simplify /btw side-question plumbing - Collapse the per-capability extraction helpers into one extractPosthogCapabilities (workspace-server) and readCapabilities (core) so future capabilities extend an object instead of cloning a cast chain. - Make extMethod dispatch symmetric: refresh_session validation moves into handleRefreshSession beside answerSideQuestion. - Move the fork's message-collection loop into collectSideQuestionAnswer in side-question.ts, flattening answerSideQuestion. - Model SideQuestionEntry as a status-discriminated union, use crypto.randomUUID for entry ids, and extract the fire-and-forget saga into fireSideQuestion beside the store, using getErrorMessage so tRPC error shapes surface their real message. - Clear withTimeout's timer once the race settles so a successful side question doesn't leave a 120s timer alive. Generated-By: PostHog Code Task-Id: c8fbfbf7-10b8-4d4b-8088-c0e9d76075e9 --- .../agent/src/adapters/claude/claude-agent.ts | 33 ++----- .../src/adapters/claude/side-question.ts | 33 ++++++- .../packages/agent/src/utils/common.ts | 13 ++- .../core/src/sessions/sessionService.ts | 22 +++-- .../sessions/components/SideQuestionCard.tsx | 4 +- .../sessions/hooks/useSessionCallbacks.ts | 22 +---- .../features/sessions/sideQuestionStore.ts | 87 ++++++++++++------- .../src/services/agent/agent.ts | 42 ++++----- 8 files changed, 140 insertions(+), 116 deletions(-) diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts index 37809ea4ee35..b1bb908bb6db 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts @@ -148,6 +148,7 @@ import { import { SettingsManager } from "./session/settings"; import { buildSideQuestionPrompt, + collectSideQuestionAnswer, SIDE_QUESTION_TIMEOUT_MS, } from "./side-question"; import { @@ -1642,11 +1643,15 @@ export class ClaudeAcpAgent extends BaseAcpAgent { } return await this.answerSideQuestion(params.question); } - - if (!isMethod(method, POSTHOG_METHODS.REFRESH_SESSION)) { - throw RequestError.methodNotFound(method); + if (isMethod(method, POSTHOG_METHODS.REFRESH_SESSION)) { + return await this.handleRefreshSession(params); } + throw RequestError.methodNotFound(method); + } + private async handleRefreshSession( + params: Record, + ): Promise> { // Trust boundary: refresh is only safe when the caller is trusted infra // (e.g. the sandbox agent-server). Do not route this method from // untrusted clients — parseMcpServers does no URL/command validation. @@ -2020,27 +2025,7 @@ export class ClaudeAcpAgent extends BaseAcpAgent { }); const answer = await withTimeout( - (async () => { - const chunks: string[] = []; - for await (const message of oneShot) { - if (message.type === "assistant") { - for (const block of message.message.content) { - if (block.type === "text") { - chunks.push(block.text); - } - } - } else if (message.type === "result") { - if (message.subtype !== "success") { - throw new RequestError( - -32603, - `Side question failed: ${message.subtype}`, - ); - } - break; - } - } - return chunks.join("").trim(); - })(), + collectSideQuestionAnswer(oneShot), SIDE_QUESTION_TIMEOUT_MS, ); diff --git a/products/desktop/packages/agent/src/adapters/claude/side-question.ts b/products/desktop/packages/agent/src/adapters/claude/side-question.ts index be6917625bd2..2b9cad759335 100644 --- a/products/desktop/packages/agent/src/adapters/claude/side-question.ts +++ b/products/desktop/packages/agent/src/adapters/claude/side-question.ts @@ -1,10 +1,41 @@ /** - * Helpers for the one-shot "/btw" side question. Pure functions only — + * Helpers for the one-shot "/btw" side question. Session-independent only — * the forked query mechanics live in ClaudeAcpAgent.answerSideQuestion. */ +import { RequestError } from "@agentclientprotocol/sdk"; +import type { SDKMessage } from "@anthropic-ai/claude-agent-sdk"; + export const SIDE_QUESTION_TIMEOUT_MS = 120_000; +/** + * Drains the one-shot fork's message stream and returns the assistant's + * answer text. Throws when the turn ends in an error result. + */ +export async function collectSideQuestionAnswer( + messages: AsyncIterable, +): Promise { + const chunks: string[] = []; + for await (const message of messages) { + if (message.type === "assistant") { + for (const block of message.message.content) { + if (block.type === "text") { + chunks.push(block.text); + } + } + } else if (message.type === "result") { + if (message.subtype !== "success") { + throw new RequestError( + -32603, + `Side question failed: ${message.subtype}`, + ); + } + break; + } + } + return chunks.join("").trim(); +} + /** * Wraps the user's question in the constraints the fork must obey: no tools, * single one-off response, answer only from what the conversation already diff --git a/products/desktop/packages/agent/src/utils/common.ts b/products/desktop/packages/agent/src/utils/common.ts index 867f6b2f61b7..36646bc454e8 100644 --- a/products/desktop/packages/agent/src/utils/common.ts +++ b/products/desktop/packages/agent/src/utils/common.ts @@ -9,14 +9,19 @@ export async function withTimeout( operation: Promise, timeoutMs: number, ): Promise<{ result: "success"; value: T } | { result: "timeout" }> { - const timeoutPromise = new Promise<{ result: "timeout" }>((resolve) => - setTimeout(() => resolve({ result: "timeout" }), timeoutMs), - ); + let timer: ReturnType | undefined; + const timeoutPromise = new Promise<{ result: "timeout" }>((resolve) => { + timer = setTimeout(() => resolve({ result: "timeout" }), timeoutMs); + }); const operationPromise = operation.then((value) => ({ result: "success" as const, value, })); - return Promise.race([operationPromise, timeoutPromise]); + try { + return await Promise.race([operationPromise, timeoutPromise]); + } finally { + clearTimeout(timer); + } } /** diff --git a/products/desktop/packages/core/src/sessions/sessionService.ts b/products/desktop/packages/core/src/sessions/sessionService.ts index 60acb97c7190..1acd836cb7ae 100644 --- a/products/desktop/packages/core/src/sessions/sessionService.ts +++ b/products/desktop/packages/core/src/sessions/sessionService.ts @@ -1673,14 +1673,14 @@ export function isPermissionRequestAlreadySurfaced( ); } -/** The steering capability on a loosely-typed agent start/reconnect result. */ -function readSteering(result: unknown): string | undefined { - return (result as { steering?: string } | undefined)?.steering; -} - -/** The side-question capability on a loosely-typed agent start/reconnect result. */ -function readSideQuestion(result: unknown): boolean | undefined { - return (result as { sideQuestion?: boolean } | undefined)?.sideQuestion; +/** The negotiated capabilities on a loosely-typed agent start/reconnect result. */ +function readCapabilities(result: unknown): { + steering?: string; + sideQuestion?: boolean; +} { + const { steering, sideQuestion } = + (result as { steering?: string; sideQuestion?: boolean } | undefined) ?? {}; + return { steering, sideQuestion }; } // Live streaming emits agent_message_chunk; SessionLogWriter coalesces a chunk @@ -2334,8 +2334,7 @@ export class SessionService { this.d.store.updateSession(taskRunId, { status: "connected", configOptions, - steering: readSteering(result), - sideQuestion: readSideQuestion(result), + ...readCapabilities(result), }); // Persist the merged config options @@ -2692,8 +2691,7 @@ export class SessionService { | SessionConfigOption[] | undefined; session.configOptions = configOptions; - session.steering = readSteering(result); - session.sideQuestion = readSideQuestion(result); + Object.assign(session, readCapabilities(result)); // Persist the config options if (configOptions) { diff --git a/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx b/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx index 08dc3a2e017c..978bb6ff46d8 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx @@ -50,9 +50,7 @@ export function SideQuestionCard({ taskId }: SideQuestionCardProps) { )} {entry.status === "error" && ( - - {entry.error ?? "Side question failed"} - + {entry.error} )} diff --git a/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts b/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts index 655798177d98..26accdb43f9d 100644 --- a/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts +++ b/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts @@ -26,7 +26,7 @@ import { type AgentSession, sessionStoreSetters, } from "@posthog/ui/features/sessions/sessionStore"; -import { useSideQuestionStore } from "@posthog/ui/features/sessions/sideQuestionStore"; +import { fireSideQuestion } from "@posthog/ui/features/sessions/sideQuestionStore"; import { useTaskViewed } from "@posthog/ui/features/sidebar/useTaskViewed"; import { SHELL_CLIENT, @@ -78,27 +78,9 @@ export function useSessionCallbacks({ } : null, taskRun: task.latest_run ?? null, - // Fire-and-forget so the composer clears immediately; the side - // question runs beside the conversation, mid-turn or idle. askSideQuestion: currentSession && sessionSupportsSideQuestion(currentSession) - ? (question) => { - const { ask, resolve, fail } = useSideQuestionStore.getState(); - const id = ask(taskId, question); - sessionService - .askSideQuestion(taskId, question) - .then((answer) => resolve(taskId, id, answer)) - .catch((error) => { - fail( - taskId, - id, - error instanceof Error - ? error.message - : "Side question failed", - ); - log.error("Side question failed", error); - }); - } + ? (question) => fireSideQuestion(sessionService, taskId, question) : undefined, }); if (handled) return true; diff --git a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts index 47f7e6b81480..bdc53c33ef9e 100644 --- a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts +++ b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts @@ -1,14 +1,15 @@ +import type { SessionService } from "@posthog/core/sessions/sessionService"; +import { getErrorMessage } from "@posthog/shared"; +import { logger } from "@posthog/ui/shell/logger"; import { create } from "zustand"; -export type SideQuestionStatus = "pending" | "done" | "error"; +const log = logger.scope("side-question"); -export interface SideQuestionEntry { - id: string; - question: string; - answer: string; - status: SideQuestionStatus; - error?: string; -} +export type SideQuestionEntry = { id: string; question: string } & ( + | { status: "pending" } + | { status: "done"; answer: string } + | { status: "error"; error: string } +); interface SideQuestionState { /** Latest side question per task. Ephemeral: never persisted, never part of session history. */ @@ -19,45 +20,69 @@ interface SideQuestionState { dismiss: (taskId: string) => void; } -let nextId = 0; +/** + * Settles the entry only if it is still the one the caller created — a + * re-asked or dismissed question must not receive a stale answer. + */ +function settle( + state: SideQuestionState, + taskId: string, + id: string, + outcome: + | { status: "done"; answer: string } + | { status: "error"; error: string }, +): Partial | SideQuestionState { + const entry = state.byTaskId[taskId]; + if (entry?.id !== id) return state; + return { + byTaskId: { + ...state.byTaskId, + [taskId]: { id: entry.id, question: entry.question, ...outcome }, + }, + }; +} export const useSideQuestionStore = create()((set) => ({ byTaskId: {}, ask: (taskId, question) => { - const id = `sq-${++nextId}`; + const id = crypto.randomUUID(); set((state) => ({ byTaskId: { ...state.byTaskId, - [taskId]: { id, question, answer: "", status: "pending" }, + [taskId]: { id, question, status: "pending" }, }, })); return id; }, resolve: (taskId, id, answer) => - set((state) => { - const entry = state.byTaskId[taskId]; - if (entry?.id !== id) return state; - return { - byTaskId: { - ...state.byTaskId, - [taskId]: { ...entry, answer, status: "done" }, - }, - }; - }), + set((state) => settle(state, taskId, id, { status: "done", answer })), fail: (taskId, id, error) => - set((state) => { - const entry = state.byTaskId[taskId]; - if (entry?.id !== id) return state; - return { - byTaskId: { - ...state.byTaskId, - [taskId]: { ...entry, error, status: "error" }, - }, - }; - }), + set((state) => settle(state, taskId, id, { status: "error", error })), dismiss: (taskId) => set((state) => { const { [taskId]: _dismissed, ...rest } = state.byTaskId; return { byTaskId: rest }; }), })); + +/** + * Fire-and-forget "/btw" saga: record the pending entry, send the question, + * and settle the entry with the answer or error. Fire-and-forget so the + * composer clears immediately; the side question runs beside the + * conversation, mid-turn or idle. + */ +export function fireSideQuestion( + sessionService: Pick, + taskId: string, + question: string, +): void { + const { ask, resolve, fail } = useSideQuestionStore.getState(); + const id = ask(taskId, question); + sessionService + .askSideQuestion(taskId, question) + .then((answer) => resolve(taskId, id, answer)) + .catch((error) => { + fail(taskId, id, getErrorMessage(error) || "Side question failed"); + log.error("Side question failed", error); + }); +} diff --git a/products/desktop/packages/workspace-server/src/services/agent/agent.ts b/products/desktop/packages/workspace-server/src/services/agent/agent.ts index 4f659599f42e..9c72fd212653 100644 --- a/products/desktop/packages/workspace-server/src/services/agent/agent.ts +++ b/products/desktop/packages/workspace-server/src/services/agent/agent.ts @@ -304,29 +304,29 @@ interface SessionConfig { bedrockGatewayVariant?: BedrockGatewayVariant; } -/** Pull the adapter's `agentCapabilities._meta.posthog.steering` from initialize. */ -function extractSteeringCapability(init: unknown): string | undefined { - const steering = ( +/** Pull the adapter's negotiated `agentCapabilities._meta.posthog` capabilities from initialize. */ +function extractPosthogCapabilities(init: unknown): { + steering?: string; + sideQuestion?: boolean; +} { + const posthog = ( init as { - agentCapabilities?: { _meta?: { posthog?: { steering?: unknown } } }; + agentCapabilities?: { _meta?: { posthog?: Record } }; } - )?.agentCapabilities?._meta?.posthog?.steering; - return typeof steering === "string" ? steering : undefined; + )?.agentCapabilities?._meta?.posthog; + return { + steering: + typeof posthog?.steering === "string" ? posthog.steering : undefined, + sideQuestion: + typeof posthog?.sideQuestion === "boolean" + ? posthog.sideQuestion + : undefined, + }; } /** A streaming turn emits many events a second; warn once a minute, not per event. */ const NO_LISTENER_WARN_INTERVAL_MS = 60_000; -/** Pull the adapter's `agentCapabilities._meta.posthog.sideQuestion` from initialize. */ -function extractSideQuestionCapability(init: unknown): boolean | undefined { - const sideQuestion = ( - init as { - agentCapabilities?: { _meta?: { posthog?: { sideQuestion?: unknown } } }; - } - )?.agentCapabilities?._meta?.posthog?.sideQuestion; - return typeof sideQuestion === "boolean" ? sideQuestion : undefined; -} - interface ManagedSession { taskRunId: string; taskId: string; @@ -956,11 +956,11 @@ If a repository is required, call \`list_repos\` to find it, then use \`clone_re }, }); // The adapter advertises whether mid-turn steering folds natively into the - // running turn (`steering: "native"`) vs needs cancel+resend. Surface it so - // the host gates steer-vs-resend on the negotiated capability, not on a - // hardcoded adapter name (codex-acp advertises "interrupt-resend"). - const steering = extractSteeringCapability(initResult); - const sideQuestion = extractSideQuestionCapability(initResult); + // running turn (`steering: "native"`) vs needs cancel+resend, and whether + // it can answer one-shot "/btw" side questions. Surface both so the host + // gates on the negotiated capabilities, not on a hardcoded adapter name + // (codex-acp advertises "interrupt-resend"). + const { steering, sideQuestion } = extractPosthogCapabilities(initResult); const { servers: mcpServers, From 45620363d07a69584bf08919e270424c71eb81a2 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:18 -0700 Subject: [PATCH 06/15] fix: address /btw side-question review findings - Correct AgentService.sideQuestion's doc comment: it does reset the session's idle-kill timer via recordActivity, contrary to what the comment claimed - Rename the dropped sessionId destructure target from _drop to _sessionId for clarity - Add an aria-live region to SideQuestionCard so the answer is announced to screen readers - Add a title tooltip so a truncated question is still readable on hover - Add test coverage: the in-flight guard clearing after a failed side question, the stale-answer guard in sideQuestionStore, the SideQuestionCard render states and dismiss button, the askSideQuestion gating in useSessionCallbacks, and AgentService.sideQuestion end to end --- .../claude/claude-agent.side-question.test.ts | 18 ++++ .../agent/src/adapters/claude/claude-agent.ts | 2 +- .../components/SideQuestionCard.test.tsx | 97 +++++++++++++++++++ .../sessions/components/SideQuestionCard.tsx | 7 +- .../hooks/useSessionCallbacks.test.tsx | 69 ++++++++++++- .../sessions/sideQuestionStore.test.ts | 30 ++++++ .../src/services/agent/agent.test.ts | 80 +++++++++++++++ .../src/services/agent/agent.ts | 7 +- 8 files changed, 302 insertions(+), 8 deletions(-) create mode 100644 products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.test.tsx create mode 100644 products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts index a5abc64ab6ab..7dcc03783ae9 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts @@ -230,6 +230,24 @@ describe("ClaudeAcpAgent.extMethod side_question", () => { ).resolves.toEqual({ answer: "third" }); }); + it("clears the in-flight guard after a failed side question", async () => { + const agent = makeAgent(); + installFakeSession(agent, "s-8"); + + nextQueryMessages = [{ type: "result", subtype: "error_max_turns" }]; + await expect( + agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "first?" }), + ).rejects.toBeInstanceOf(RequestError); + + nextQueryMessages = [ + assistantText("second"), + { type: "result", subtype: "success" }, + ]; + await expect( + agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "second?" }), + ).resolves.toEqual({ answer: "second" }); + }); + it("wraps SDK failures (e.g. nothing to resume) in a clear RequestError", async () => { const agent = makeAgent(); installFakeSession(agent, "s-7"); diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts index b1bb908bb6db..45be62286b07 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts @@ -1990,7 +1990,7 @@ export class ClaudeAcpAgent extends BaseAcpAgent { // Drop `sessionId` (identity comes from `resume`) and `hooks` (they // close over live-session caches and task state). const { - sessionId: _drop, + sessionId: _sessionId, hooks: _hooks, ...rest } = this.session.queryOptions; diff --git a/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.test.tsx b/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.test.tsx new file mode 100644 index 000000000000..9c4f6bbadd60 --- /dev/null +++ b/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.test.tsx @@ -0,0 +1,97 @@ +import { Theme } from "@radix-ui/themes"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { beforeEach, describe, expect, it } from "vitest"; +import { useSideQuestionStore } from "../sideQuestionStore"; +import { SideQuestionCard } from "./SideQuestionCard"; + +describe("SideQuestionCard", () => { + beforeEach(() => { + useSideQuestionStore.setState({ byTaskId: {} }); + }); + + it("renders nothing when there is no entry for the task", () => { + const { container } = render( + + + , + ); + expect(container.firstElementChild).toBeEmptyDOMElement(); + }); + + it("shows a spinner while the question is pending", () => { + useSideQuestionStore.setState({ + byTaskId: { + "task-1": { id: "q-1", question: "what changed?", status: "pending" }, + }, + }); + + render( + + + , + ); + + expect(screen.getByText("what changed?")).toBeInTheDocument(); + expect(screen.getByText("Answering…")).toBeInTheDocument(); + }); + + it("renders the answer once done", () => { + useSideQuestionStore.setState({ + byTaskId: { + "task-1": { + id: "q-1", + question: "what changed?", + status: "done", + answer: "The function parses JSONL.", + }, + }, + }); + + render( + + + , + ); + + expect(screen.getByText("The function parses JSONL.")).toBeInTheDocument(); + }); + + it("renders the error message on failure", () => { + useSideQuestionStore.setState({ + byTaskId: { + "task-1": { + id: "q-1", + question: "what changed?", + status: "error", + error: "Side question timed out", + }, + }, + }); + + render( + + + , + ); + + expect(screen.getByText("Side question timed out")).toBeInTheDocument(); + }); + + it("dismisses the entry when the dismiss button is clicked", () => { + useSideQuestionStore.setState({ + byTaskId: { + "task-1": { id: "q-1", question: "what changed?", status: "pending" }, + }, + }); + + render( + + + , + ); + + fireEvent.click(screen.getByLabelText("Dismiss side question")); + + expect(useSideQuestionStore.getState().byTaskId["task-1"]).toBeUndefined(); + }); +}); diff --git a/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx b/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx index 978bb6ff46d8..572fcf59fd9d 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx @@ -22,7 +22,10 @@ export function SideQuestionCard({ taskId }: SideQuestionCardProps) { - + {entry.question} @@ -37,7 +40,7 @@ export function SideQuestionCard({ taskId }: SideQuestionCardProps) { - + {entry.status === "pending" && ( diff --git a/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.test.tsx b/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.test.tsx index 1bb918f22469..b10d85beb34e 100644 --- a/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.test.tsx +++ b/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.test.tsx @@ -48,9 +48,13 @@ vi.mock("@posthog/ui/features/sessions/hooks/useMessagingMode", () => ({ useMessagingMode: () => messagingMode.value, })); -// No code command / skill rewrite; the raw text is used as the prompt. +// No code command / skill rewrite by default; the raw text is used as the +// prompt. Kept as a spy so tests can inspect the CommandContext it received. +const tryExecuteCodeCommand = vi.hoisted(() => + vi.fn(async (_text: string, _ctx: Record) => false), +); vi.mock("@posthog/ui/features/message-editor/commands", () => ({ - tryExecuteCodeCommand: async () => false, + tryExecuteCodeCommand, rewriteLocalSkillCommandPrompt: () => null, resolveLocalSkillPrompt: async (text: string) => text, })); @@ -103,6 +107,27 @@ function renderCallbacks(session?: AgentSession) { ); } +function makeSession(overrides: Partial): AgentSession { + return { + taskRunId: "run-1", + taskId: TASK, + taskTitle: "Test", + channel: "agent-event:run-1", + events: [], + startedAt: 0, + status: "connected", + isPromptPending: false, + isCompacting: false, + promptStartedAt: null, + pendingPermissions: new Map(), + pausedDurationMs: 0, + messageQueue: [], + optimisticItems: [], + adapter: "claude", + ...overrides, + } as AgentSession; +} + describe("useSessionCallbacks.handleSendPrompt while editing a queued message", () => { beforeEach(() => { vi.clearAllMocks(); @@ -269,3 +294,43 @@ describe("useSessionCallbacks.handleCancelPrompt", () => { }); }); }); + +describe("useSessionCallbacks.handleSendPrompt askSideQuestion gating", () => { + beforeEach(() => { + vi.clearAllMocks(); + sessionState.editingQueuedId = undefined; + sessionState.messageQueue = []; + }); + + it("passes a callable askSideQuestion for a session that supports it", async () => { + const { result } = renderHook(() => + useSessionCallbacks({ + taskId: TASK, + task, + session: makeSession({ isCloud: false, sideQuestion: true }), + repoPath: "/repo", + }), + ); + await result.current.handleSendPrompt("/btw what changed?"); + + const ctx = tryExecuteCodeCommand.mock.calls.at(-1)?.[1]; + expect(ctx).toBeDefined(); + expect(ctx?.askSideQuestion).toBeInstanceOf(Function); + }); + + it("passes undefined askSideQuestion for a session that does not support it", async () => { + const { result } = renderHook(() => + useSessionCallbacks({ + taskId: TASK, + task, + session: makeSession({ isCloud: true, sideQuestion: true }), + repoPath: "/repo", + }), + ); + await result.current.handleSendPrompt("/btw what changed?"); + + const ctx = tryExecuteCodeCommand.mock.calls.at(-1)?.[1]; + expect(ctx).toBeDefined(); + expect(ctx?.askSideQuestion).toBeUndefined(); + }); +}); diff --git a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts new file mode 100644 index 000000000000..96980bf91e23 --- /dev/null +++ b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts @@ -0,0 +1,30 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { useSideQuestionStore } from "./sideQuestionStore"; + +describe("sideQuestionStore stale-answer guard", () => { + beforeEach(() => { + useSideQuestionStore.setState({ byTaskId: {} }); + }); + + it("does not repopulate a dismissed entry with a late answer", () => { + const { ask, dismiss, resolve } = useSideQuestionStore.getState(); + + const id = ask("task-1", "what changed?"); + dismiss("task-1"); + resolve("task-1", id, "the late answer"); + + expect(useSideQuestionStore.getState().byTaskId["task-1"]).toBeUndefined(); + }); + + it("does not let a stale answer clobber a re-asked question", () => { + const { ask, resolve } = useSideQuestionStore.getState(); + + const firstId = ask("task-1", "first question?"); + const secondId = ask("task-1", "second question?"); + resolve("task-1", firstId, "answer to the first question"); + + const entry = useSideQuestionStore.getState().byTaskId["task-1"]; + expect(entry?.id).toBe(secondId); + expect(entry?.status).toBe("pending"); + }); +}); diff --git a/products/desktop/packages/workspace-server/src/services/agent/agent.test.ts b/products/desktop/packages/workspace-server/src/services/agent/agent.test.ts index 7cb81a311961..610a3020bba1 100644 --- a/products/desktop/packages/workspace-server/src/services/agent/agent.test.ts +++ b/products/desktop/packages/workspace-server/src/services/agent/agent.test.ts @@ -110,6 +110,10 @@ vi.mock("@agentclientprotocol/sdk", () => ({ vi.mock("@posthog/agent", () => ({ isMcpToolReadOnly: vi.fn(() => false), + POSTHOG_METHODS: { + SIDE_QUESTION: "_posthog/side_question", + REFRESH_SESSION: "_posthog/refresh_session", + }, })); vi.mock("@posthog/agent/posthog-api", () => ({ @@ -1011,6 +1015,82 @@ describe("AgentService", () => { }); }); + describe("sideQuestion", () => { + function injectSession( + svc: AgentService, + taskRunId: string, + overrides: Record = {}, + ) { + const sessions = (svc as unknown as { sessions: Map }) + .sessions; + sessions.set(taskRunId, { + taskRunId, + taskId: `task-for-${taskRunId}`, + repoPath: "/mock/repo", + agent: { cleanup: vi.fn().mockResolvedValue(undefined) }, + clientSideConnection: { extMethod: vi.fn() }, + channel: `ch-${taskRunId}`, + createdAt: Date.now(), + lastActivityAt: 0, + config: { sessionId: "agent-session-1" }, + promptPending: false, + inFlightMcpToolCalls: new Map(), + mcpToolApprovals: {}, + toolInstallations: {}, + ...overrides, + }); + } + + it("throws when no session exists for the given id", async () => { + await expect(service.sideQuestion("unknown-run", "why?")).rejects.toThrow( + "Session not found: unknown-run", + ); + }); + + it("forwards the question to the adapter and returns the parsed answer", async () => { + const extMethod = vi.fn().mockResolvedValue({ answer: "42" }); + injectSession(service, "run-1", { + clientSideConnection: { extMethod }, + }); + + const result = await service.sideQuestion("run-1", "why?"); + + expect(result).toEqual({ answer: "42" }); + expect(extMethod).toHaveBeenCalledWith("_posthog/side_question", { + sessionId: "agent-session-1", + question: "why?", + }); + }); + + it("records activity on the session", async () => { + const extMethod = vi.fn().mockResolvedValue({ answer: "42" }); + injectSession(service, "run-1", { + clientSideConnection: { extMethod }, + lastActivityAt: 0, + }); + const recordActivitySpy = vi.spyOn(service, "recordActivity"); + + await service.sideQuestion("run-1", "why?"); + + expect(recordActivitySpy).toHaveBeenCalledWith("run-1"); + const sessions = ( + service as unknown as { + sessions: Map; + } + ).sessions; + expect(sessions.get("run-1")?.lastActivityAt).toBeGreaterThan(0); + }); + + it("throws when the adapter returns a malformed result", async () => { + const extMethod = vi.fn().mockResolvedValue({ notAnAnswer: true }); + injectSession(service, "run-1", { + clientSideConnection: { extMethod }, + }); + + await expect(service.sideQuestion("run-1", "why?")).rejects.toThrow(); + }); + }); + describe("channel system prompt repository isolation", () => { const credentials = { apiHost: "https://app.posthog.com", projectId: 1 }; diff --git a/products/desktop/packages/workspace-server/src/services/agent/agent.ts b/products/desktop/packages/workspace-server/src/services/agent/agent.ts index 9c72fd212653..bb04af25a436 100644 --- a/products/desktop/packages/workspace-server/src/services/agent/agent.ts +++ b/products/desktop/packages/workspace-server/src/services/agent/agent.ts @@ -1476,9 +1476,10 @@ If a repository is required, call \`list_repos\` to find it, then use \`clone_re /** * Answers a one-shot "/btw" side question via the adapter's SIDE_QUESTION - * extension method. Deliberately does not touch promptPending or the - * sleep/idle lifecycle: the exchange runs beside the conversation (ACP - * JSON-RPC multiplexes, so this works mid-turn) and never becomes part of it. + * extension method. Never touches promptPending and never becomes part of + * the conversation; it does count as activity (resets the idle-kill timer), + * the same way refreshSession does. The exchange runs beside the + * conversation (ACP JSON-RPC multiplexes, so this works mid-turn). */ async sideQuestion( sessionId: string, From 2451d8b50ed352144a9719d8be7f7a6fa216eebb Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:20 -0700 Subject: [PATCH 07/15] fix: scope side-question cards to their run and keep sessions alive during them Key answers and cards by taskRunId so a completed /btw answer never displays under a session that reconnected or started a new run. Track pending side questions on the managed session so the idle timer does not reap a session while a side question is still awaiting its answer. --- .../sessions/components/SessionView.tsx | 8 ++- .../components/SideQuestionCard.test.tsx | 48 +++++++++++-- .../sessions/components/SideQuestionCard.tsx | 6 +- .../sessions/hooks/useSessionCallbacks.ts | 8 ++- .../sessions/sideQuestionStore.test.ts | 20 ++++-- .../features/sessions/sideQuestionStore.ts | 61 ++++++++++++----- .../src/services/agent/agent.test.ts | 67 +++++++++++++++++++ .../src/services/agent/agent.ts | 29 ++++++-- 8 files changed, 208 insertions(+), 39 deletions(-) diff --git a/products/desktop/packages/ui/src/features/sessions/components/SessionView.tsx b/products/desktop/packages/ui/src/features/sessions/components/SessionView.tsx index f224d22339bc..2ab22f9950bb 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/SessionView.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/SessionView.tsx @@ -409,6 +409,7 @@ export function SessionView({ (s) => !!s?.editingQueuedId, ); const cancelQueuedEdit = useCancelQueuedMessageEdit(taskId); + const activeTaskRunId = useSessionSelector(taskId, (s) => s?.taskRunId); const [isDraggingFile, setIsDraggingFile] = useState(false); const promptRecallRef = useRef(null); @@ -741,7 +742,12 @@ export function SessionView({ }`} > - {taskId && } + {taskId && ( + + )} {taskId && } { it("renders nothing when there is no entry for the task", () => { const { container } = render( - + , ); expect(container.firstElementChild).toBeEmptyDOMElement(); @@ -21,13 +21,18 @@ describe("SideQuestionCard", () => { it("shows a spinner while the question is pending", () => { useSideQuestionStore.setState({ byTaskId: { - "task-1": { id: "q-1", question: "what changed?", status: "pending" }, + "task-1": { + id: "q-1", + question: "what changed?", + taskRunId: "run-1", + status: "pending", + }, }, }); render( - + , ); @@ -41,6 +46,7 @@ describe("SideQuestionCard", () => { "task-1": { id: "q-1", question: "what changed?", + taskRunId: "run-1", status: "done", answer: "The function parses JSONL.", }, @@ -49,7 +55,7 @@ describe("SideQuestionCard", () => { render( - + , ); @@ -62,6 +68,7 @@ describe("SideQuestionCard", () => { "task-1": { id: "q-1", question: "what changed?", + taskRunId: "run-1", status: "error", error: "Side question timed out", }, @@ -70,23 +77,50 @@ describe("SideQuestionCard", () => { render( - + , ); expect(screen.getByText("Side question timed out")).toBeInTheDocument(); }); + it("renders nothing when the entry belongs to a prior run", () => { + useSideQuestionStore.setState({ + byTaskId: { + "task-1": { + id: "q-1", + question: "what changed?", + taskRunId: "run-1", + status: "done", + answer: "The function parses JSONL.", + }, + }, + }); + + const { container } = render( + + + , + ); + + expect(container.firstElementChild).toBeEmptyDOMElement(); + }); + it("dismisses the entry when the dismiss button is clicked", () => { useSideQuestionStore.setState({ byTaskId: { - "task-1": { id: "q-1", question: "what changed?", status: "pending" }, + "task-1": { + id: "q-1", + question: "what changed?", + taskRunId: "run-1", + status: "pending", + }, }, }); render( - + , ); diff --git a/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx b/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx index 572fcf59fd9d..5d2386796695 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/SideQuestionCard.tsx @@ -5,6 +5,8 @@ import { MarkdownRenderer } from "../../editor/components/MarkdownRenderer"; interface SideQuestionCardProps { taskId: string; + /** The task's current run. An entry asked against a prior run is hidden. */ + taskRunId?: string; } /** @@ -12,11 +14,11 @@ interface SideQuestionCardProps { * lives only in view state — it is never part of the session transcript — so * dismissing it leaves no trace. */ -export function SideQuestionCard({ taskId }: SideQuestionCardProps) { +export function SideQuestionCard({ taskId, taskRunId }: SideQuestionCardProps) { const entry = useSideQuestionStore((s) => s.byTaskId[taskId]); const dismiss = useSideQuestionStore((s) => s.dismiss); - if (!entry) return null; + if (!entry || entry.taskRunId !== taskRunId) return null; return ( diff --git a/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts b/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts index 26accdb43f9d..c081e0b96b7d 100644 --- a/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts +++ b/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.ts @@ -80,7 +80,13 @@ export function useSessionCallbacks({ taskRun: task.latest_run ?? null, askSideQuestion: currentSession && sessionSupportsSideQuestion(currentSession) - ? (question) => fireSideQuestion(sessionService, taskId, question) + ? (question) => + fireSideQuestion( + sessionService, + taskId, + currentSession.taskRunId, + question, + ) : undefined, }); if (handled) return true; diff --git a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts index 96980bf91e23..f9da26dd3544 100644 --- a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts +++ b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts @@ -9,9 +9,9 @@ describe("sideQuestionStore stale-answer guard", () => { it("does not repopulate a dismissed entry with a late answer", () => { const { ask, dismiss, resolve } = useSideQuestionStore.getState(); - const id = ask("task-1", "what changed?"); + const id = ask("task-1", "run-1", "what changed?"); dismiss("task-1"); - resolve("task-1", id, "the late answer"); + resolve("task-1", "run-1", id, "the late answer"); expect(useSideQuestionStore.getState().byTaskId["task-1"]).toBeUndefined(); }); @@ -19,12 +19,22 @@ describe("sideQuestionStore stale-answer guard", () => { it("does not let a stale answer clobber a re-asked question", () => { const { ask, resolve } = useSideQuestionStore.getState(); - const firstId = ask("task-1", "first question?"); - const secondId = ask("task-1", "second question?"); - resolve("task-1", firstId, "answer to the first question"); + const firstId = ask("task-1", "run-1", "first question?"); + const secondId = ask("task-1", "run-1", "second question?"); + resolve("task-1", "run-1", firstId, "answer to the first question"); const entry = useSideQuestionStore.getState().byTaskId["task-1"]; expect(entry?.id).toBe(secondId); expect(entry?.status).toBe("pending"); }); + + it("does not settle an answer against a run the task has since moved on from", () => { + const { ask, resolve } = useSideQuestionStore.getState(); + + const id = ask("task-1", "run-1", "what changed?"); + resolve("task-1", "run-2", id, "answer from the old run"); + + const entry = useSideQuestionStore.getState().byTaskId["task-1"]; + expect(entry?.status).toBe("pending"); + }); }); diff --git a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts index bdc53c33ef9e..a308e9763e16 100644 --- a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts +++ b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts @@ -5,7 +5,12 @@ import { create } from "zustand"; const log = logger.scope("side-question"); -export type SideQuestionEntry = { id: string; question: string } & ( +export type SideQuestionEntry = { + id: string; + question: string; + /** The run this question was asked against — a card must not outlive it. */ + taskRunId: string; +} & ( | { status: "pending" } | { status: "done"; answer: string } | { status: "error"; error: string } @@ -14,50 +19,66 @@ export type SideQuestionEntry = { id: string; question: string } & ( interface SideQuestionState { /** Latest side question per task. Ephemeral: never persisted, never part of session history. */ byTaskId: Record; - ask: (taskId: string, question: string) => string; - resolve: (taskId: string, id: string, answer: string) => void; - fail: (taskId: string, id: string, error: string) => void; + ask: (taskId: string, taskRunId: string, question: string) => string; + resolve: ( + taskId: string, + taskRunId: string, + id: string, + answer: string, + ) => void; + fail: (taskId: string, taskRunId: string, id: string, error: string) => void; dismiss: (taskId: string) => void; } /** - * Settles the entry only if it is still the one the caller created — a - * re-asked or dismissed question must not receive a stale answer. + * Settles the entry only if it is still the one the caller created for the + * same run — a re-asked or dismissed question, or a run that reconnected or + * restarted in the meantime, must not receive a stale answer. */ function settle( state: SideQuestionState, taskId: string, + taskRunId: string, id: string, outcome: | { status: "done"; answer: string } | { status: "error"; error: string }, ): Partial | SideQuestionState { const entry = state.byTaskId[taskId]; - if (entry?.id !== id) return state; + if (entry?.id !== id || entry.taskRunId !== taskRunId) return state; return { byTaskId: { ...state.byTaskId, - [taskId]: { id: entry.id, question: entry.question, ...outcome }, + [taskId]: { + id: entry.id, + question: entry.question, + taskRunId, + ...outcome, + }, }, }; } export const useSideQuestionStore = create()((set) => ({ byTaskId: {}, - ask: (taskId, question) => { + ask: (taskId, taskRunId, question) => { const id = crypto.randomUUID(); set((state) => ({ byTaskId: { ...state.byTaskId, - [taskId]: { id, question, status: "pending" }, + [taskId]: { id, question, taskRunId, status: "pending" }, }, })); return id; }, - resolve: (taskId, id, answer) => - set((state) => settle(state, taskId, id, { status: "done", answer })), - fail: (taskId, id, error) => - set((state) => settle(state, taskId, id, { status: "error", error })), + resolve: (taskId, taskRunId, id, answer) => + set((state) => + settle(state, taskId, taskRunId, id, { status: "done", answer }), + ), + fail: (taskId, taskRunId, id, error) => + set((state) => + settle(state, taskId, taskRunId, id, { status: "error", error }), + ), dismiss: (taskId) => set((state) => { const { [taskId]: _dismissed, ...rest } = state.byTaskId; @@ -74,15 +95,21 @@ export const useSideQuestionStore = create()((set) => ({ export function fireSideQuestion( sessionService: Pick, taskId: string, + taskRunId: string, question: string, ): void { const { ask, resolve, fail } = useSideQuestionStore.getState(); - const id = ask(taskId, question); + const id = ask(taskId, taskRunId, question); sessionService .askSideQuestion(taskId, question) - .then((answer) => resolve(taskId, id, answer)) + .then((answer) => resolve(taskId, taskRunId, id, answer)) .catch((error) => { - fail(taskId, id, getErrorMessage(error) || "Side question failed"); + fail( + taskId, + taskRunId, + id, + getErrorMessage(error) || "Side question failed", + ); log.error("Side question failed", error); }); } diff --git a/products/desktop/packages/workspace-server/src/services/agent/agent.test.ts b/products/desktop/packages/workspace-server/src/services/agent/agent.test.ts index 610a3020bba1..ed7b4a9a07d0 100644 --- a/products/desktop/packages/workspace-server/src/services/agent/agent.test.ts +++ b/products/desktop/packages/workspace-server/src/services/agent/agent.test.ts @@ -859,6 +859,7 @@ describe("AgentService", () => { config: {}, promptPending: false, inFlightMcpToolCalls: new Map(), + pendingSideQuestions: 0, mcpToolApprovals: {}, toolInstallations: {}, ...overrides, @@ -997,6 +998,31 @@ describe("AgentService", () => { ); }); + it("reschedules when pendingSideQuestions is non-zero at timeout", () => { + injectSession(service, "run-1", { pendingSideQuestions: 1 }); + service.recordActivity("run-1"); + + vi.advanceTimersByTime(15 * 60 * 1000); + + expect(service.emit).not.toHaveBeenCalledWith( + "session-idle-killed", + expect.anything(), + ); + expect(getIdleTimeouts(service).has("run-1")).toBe(true); + }); + + it("kills session when pendingSideQuestions is zero", () => { + injectSession(service, "run-1", { pendingSideQuestions: 0 }); + service.recordActivity("run-1"); + + vi.advanceTimersByTime(15 * 60 * 1000); + + expect(service.emit).toHaveBeenCalledWith( + "session-idle-killed", + expect.objectContaining({ taskRunId: "run-1" }), + ); + }); + it("checkIdleDeadlines does not kill non-expired sessions", () => { injectSession(service, "run-1"); service.recordActivity("run-1"); @@ -1035,6 +1061,7 @@ describe("AgentService", () => { config: { sessionId: "agent-session-1" }, promptPending: false, inFlightMcpToolCalls: new Map(), + pendingSideQuestions: 0, mcpToolApprovals: {}, toolInstallations: {}, ...overrides, @@ -1089,6 +1116,46 @@ describe("AgentService", () => { await expect(service.sideQuestion("run-1", "why?")).rejects.toThrow(); }); + + it("tracks pendingSideQuestions while the extension call is in flight", async () => { + let resolveExtMethod: (value: { answer: string }) => void = () => {}; + const extMethod = vi.fn( + () => + new Promise((resolve) => { + resolveExtMethod = resolve; + }), + ); + injectSession(service, "run-1", { clientSideConnection: { extMethod } }); + const sessions = ( + service as unknown as { + sessions: Map; + } + ).sessions; + + const promise = service.sideQuestion("run-1", "why?"); + expect(sessions.get("run-1")?.pendingSideQuestions).toBe(1); + + resolveExtMethod({ answer: "42" }); + await promise; + + expect(sessions.get("run-1")?.pendingSideQuestions).toBe(0); + }); + + it("decrements pendingSideQuestions when the extension call fails", async () => { + const extMethod = vi.fn().mockRejectedValue(new Error("boom")); + injectSession(service, "run-1", { clientSideConnection: { extMethod } }); + const sessions = ( + service as unknown as { + sessions: Map; + } + ).sessions; + + await expect(service.sideQuestion("run-1", "why?")).rejects.toThrow( + "boom", + ); + + expect(sessions.get("run-1")?.pendingSideQuestions).toBe(0); + }); }); describe("channel system prompt repository isolation", () => { diff --git a/products/desktop/packages/workspace-server/src/services/agent/agent.ts b/products/desktop/packages/workspace-server/src/services/agent/agent.ts index bb04af25a436..0c92c1cab1b7 100644 --- a/products/desktop/packages/workspace-server/src/services/agent/agent.ts +++ b/products/desktop/packages/workspace-server/src/services/agent/agent.ts @@ -347,6 +347,8 @@ interface ManagedSession { sideQuestion?: boolean; /** Tracks in-flight MCP tool calls (toolCallId → toolKey) for cancellation */ inFlightMcpToolCalls: Map; + /** Count of "/btw" side questions awaiting a response, so the idle timer does not reap the session mid-answer. */ + pendingSideQuestions: number; /** MCP tool approval states fetched at session start */ mcpToolApprovals: McpToolApprovals; /** Maps tool keys to their installation for backend approval updates */ @@ -603,7 +605,11 @@ export class AgentService extends TypedEventEmitter { private killIdleSession(taskRunId: string): void { const session = this.sessions.get(taskRunId); if (!session) return; - if (session.promptPending || session.inFlightMcpToolCalls.size > 0) { + if ( + session.promptPending || + session.inFlightMcpToolCalls.size > 0 || + session.pendingSideQuestions > 0 + ) { this.recordActivity(taskRunId); return; } @@ -1209,6 +1215,7 @@ If a repository is required, call \`list_repos\` to find it, then use \`clone_re steering, sideQuestion, inFlightMcpToolCalls: new Map(), + pendingSideQuestions: 0, mcpToolApprovals: toolApprovals, toolInstallations, evaluatedPrUrls: new Set(), @@ -1480,6 +1487,11 @@ If a repository is required, call \`list_repos\` to find it, then use \`clone_re * the conversation; it does count as activity (resets the idle-kill timer), * the same way refreshSession does. The exchange runs beside the * conversation (ACP JSON-RPC multiplexes, so this works mid-turn). + * + * `pendingSideQuestions` keeps the session alive if the idle timer fires + * while the extension call is still awaited — otherwise `killIdleSession` + * would see no pending prompt and no in-flight tool call and clean up the + * session out from under this request. */ async sideQuestion( sessionId: string, @@ -1492,12 +1504,17 @@ If a repository is required, call \`list_repos\` to find it, then use \`clone_re session.lastActivityAt = Date.now(); this.recordActivity(sessionId); + session.pendingSideQuestions++; - const result = await session.clientSideConnection.extMethod( - POSTHOG_METHODS.SIDE_QUESTION, - { sessionId: getAgentSessionId(session), question }, - ); - return sideQuestionOutput.parse(result); + try { + const result = await session.clientSideConnection.extMethod( + POSTHOG_METHODS.SIDE_QUESTION, + { sessionId: getAgentSessionId(session), question }, + ); + return sideQuestionOutput.parse(result); + } finally { + session.pendingSideQuestions--; + } } async cancelSession(sessionId: string): Promise { From 4760a93e40d1d9f42d72f45df89007127ccabb6d Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:22 -0700 Subject: [PATCH 08/15] fix(agent): handle a second /btw asked while one is pending Asking a second side question replaced the pending entry, so the card showed the second request's "already in progress" error while the first question's answer arrived to a stale-answer guard that discarded it. The store now refuses a second question for the same task and run, and /btw says so instead of swallowing it. Dismissing the card or starting a new run still lets a question through while the previous request is in flight, so the adapter aborts the fork it supersedes rather than rejecting the newer question. --- .../claude/claude-agent.side-question.test.ts | 37 ++++------- .../agent/src/adapters/claude/claude-agent.ts | 21 ++++--- .../features/message-editor/commands.test.ts | 17 +++++- .../src/features/message-editor/commands.ts | 7 ++- .../sessions/sideQuestionStore.test.ts | 61 ++++++++++++++++--- .../features/sessions/sideQuestionStore.ts | 16 ++++- 6 files changed, 110 insertions(+), 49 deletions(-) diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts index 7dcc03783ae9..7856b0606801 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts @@ -196,7 +196,7 @@ describe("ClaudeAcpAgent.extMethod side_question", () => { ).rejects.toThrow(/no answer/); }); - it("allows only one side question at a time", async () => { + it("answers a newer side question and aborts the one it supersedes", async () => { const agent = makeAgent(); installFakeSession(agent, "s-6"); @@ -212,33 +212,13 @@ describe("ClaudeAcpAgent.extMethod side_question", () => { const first = agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "first?", }); - await expect( - agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "second?" }), - ).rejects.toThrow(/already in progress/); - - release(); - await expect(first).resolves.toEqual({ answer: "first" }); + await vi.waitFor(() => + expect(lastQueryCall.options?.abortController).toBeDefined(), + ); + const firstAbort = lastQueryCall.options + ?.abortController as AbortController; - // The guard clears once the first completes. nextQueryGate = null; - nextQueryMessages = [ - assistantText("third"), - { type: "result", subtype: "success" }, - ]; - await expect( - agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "third?" }), - ).resolves.toEqual({ answer: "third" }); - }); - - it("clears the in-flight guard after a failed side question", async () => { - const agent = makeAgent(); - installFakeSession(agent, "s-8"); - - nextQueryMessages = [{ type: "result", subtype: "error_max_turns" }]; - await expect( - agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "first?" }), - ).rejects.toBeInstanceOf(RequestError); - nextQueryMessages = [ assistantText("second"), { type: "result", subtype: "success" }, @@ -246,6 +226,11 @@ describe("ClaudeAcpAgent.extMethod side_question", () => { await expect( agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "second?" }), ).resolves.toEqual({ answer: "second" }); + + expect(firstAbort.signal.aborted).toBe(true); + + release(); + await first.catch(() => {}); }); it("wraps SDK failures (e.g. nothing to resume) in a clear RequestError", async () => { diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts index 45be62286b07..79b7d996741f 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts @@ -356,8 +356,11 @@ export class ClaudeAcpAgent extends BaseAcpAgent { private options?: ClaudeAcpAgentOptions; private enrichment?: Enrichment; private enrichedReadCache: EnrichedReadCache = new Map(); - /** One side question at a time; bounds concurrent forks off the transcript. */ - private sideQuestionInFlight = false; + /** + * The in-flight side question's controller, so a newer question can abort it. + * Bounds concurrent forks off the transcript to one. + */ + private sideQuestionAbort: AbortController | null = null; constructor(client: AgentSideConnection, options?: ClaudeAcpAgentOptions) { super(client); @@ -1976,16 +1979,18 @@ export class ClaudeAcpAgent extends BaseAcpAgent { * the fork gets its own SDK session id and AbortController, and nothing on * `this.session` is touched, so the main conversation (including an * in-flight turn) never sees the exchange. + * + * A newer question supersedes an in-flight one: the card shows only the + * latest, so answering a question it has already replaced burns tokens on + * a result the stale-answer guard would discard anyway. */ private async answerSideQuestion( question: string, ): Promise<{ answer: string }> { - if (this.sideQuestionInFlight) { - throw new RequestError(-32002, "A side question is already in progress"); - } - this.sideQuestionInFlight = true; + this.sideQuestionAbort?.abort(); const abortController = new AbortController(); + this.sideQuestionAbort = abortController; try { // Drop `sessionId` (identity comes from `resume`) and `hooks` (they // close over live-session caches and task state). @@ -2049,7 +2054,9 @@ export class ClaudeAcpAgent extends BaseAcpAgent { ); } finally { abortController.abort(); - this.sideQuestionInFlight = false; + if (this.sideQuestionAbort === abortController) { + this.sideQuestionAbort = null; + } } } diff --git a/products/desktop/packages/ui/src/features/message-editor/commands.test.ts b/products/desktop/packages/ui/src/features/message-editor/commands.test.ts index 18cc6edd58c7..c1cccb24e30c 100644 --- a/products/desktop/packages/ui/src/features/message-editor/commands.test.ts +++ b/products/desktop/packages/ui/src/features/message-editor/commands.test.ts @@ -56,13 +56,28 @@ describe("/btw command", () => { }; it("routes the question to askSideQuestion and reports handled", async () => { - const askSideQuestion = vi.fn(); + toastError.mockClear(); + const askSideQuestion = vi.fn().mockReturnValue(true); const handled = await tryExecuteCodeCommand("/btw what changed here?", { ...baseContext, askSideQuestion, }); expect(handled).toBe(true); expect(askSideQuestion).toHaveBeenCalledWith("what changed here?"); + expect(toastError).not.toHaveBeenCalled(); + }); + + it("toasts when a side question is already pending", async () => { + toastError.mockClear(); + const askSideQuestion = vi.fn().mockReturnValue(false); + const handled = await tryExecuteCodeCommand("/btw and another thing?", { + ...baseContext, + askSideQuestion, + }); + expect(handled).toBe(true); + expect(toastError).toHaveBeenCalledWith( + "Wait for your last side question to finish first", + ); }); it("toasts on an empty question without calling askSideQuestion", async () => { diff --git a/products/desktop/packages/ui/src/features/message-editor/commands.ts b/products/desktop/packages/ui/src/features/message-editor/commands.ts index 531942ed823e..c7a63bc0217d 100644 --- a/products/desktop/packages/ui/src/features/message-editor/commands.ts +++ b/products/desktop/packages/ui/src/features/message-editor/commands.ts @@ -27,7 +27,8 @@ interface CommandContext { } | null; taskRun: { id?: string; log_url?: string } | null; /** Fires a "/btw" side question. Absent when the session doesn't support them. */ - askSideQuestion?: (question: string) => void; + /** Returns false when a side question is already pending for this run. */ + askSideQuestion?: (question: string) => boolean; } export interface CodeCommandInsertContext { @@ -121,7 +122,9 @@ const btwCommand: CodeCommand = { toast.error("Side questions aren't supported for this session yet."); return; } - ctx.askSideQuestion(question); + if (!ctx.askSideQuestion(question)) { + toast.error("Wait for your last side question to finish first"); + } }, }; diff --git a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts index f9da26dd3544..96190b424196 100644 --- a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts +++ b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.test.ts @@ -1,27 +1,68 @@ import { beforeEach, describe, expect, it } from "vitest"; import { useSideQuestionStore } from "./sideQuestionStore"; -describe("sideQuestionStore stale-answer guard", () => { +/** `ask` returns null when one is already pending; these cases expect it to take. */ +function askId(taskId: string, taskRunId: string, question: string): string { + const id = useSideQuestionStore.getState().ask(taskId, taskRunId, question); + if (!id) throw new Error(`ask unexpectedly refused: ${question}`); + return id; +} + +describe("sideQuestionStore", () => { beforeEach(() => { useSideQuestionStore.setState({ byTaskId: {} }); }); it("does not repopulate a dismissed entry with a late answer", () => { - const { ask, dismiss, resolve } = useSideQuestionStore.getState(); + const { dismiss, resolve } = useSideQuestionStore.getState(); - const id = ask("task-1", "run-1", "what changed?"); + const id = askId("task-1", "run-1", "what changed?"); dismiss("task-1"); resolve("task-1", "run-1", id, "the late answer"); expect(useSideQuestionStore.getState().byTaskId["task-1"]).toBeUndefined(); }); - it("does not let a stale answer clobber a re-asked question", () => { - const { ask, resolve } = useSideQuestionStore.getState(); + it("refuses a second question while the first is still pending", () => { + const { ask } = useSideQuestionStore.getState(); + + const firstId = askId("task-1", "run-1", "first question?"); + expect(ask("task-1", "run-1", "second question?")).toBeNull(); + + const entry = useSideQuestionStore.getState().byTaskId["task-1"]; + expect(entry?.id).toBe(firstId); + expect(entry?.question).toBe("first question?"); + }); + + it.each([ + [ + "answered", + (id: string) => + useSideQuestionStore + .getState() + .resolve("task-1", "run-1", id, "the answer"), + ], + [ + "failed", + (id: string) => + useSideQuestionStore.getState().fail("task-1", "run-1", id, "it broke"), + ], + ])("allows a new question once the previous one has %s", (_label, settle) => { + const firstId = askId("task-1", "run-1", "first question?"); + settle(firstId); + + expect( + useSideQuestionStore.getState().ask("task-1", "run-1", "second?"), + ).not.toBeNull(); + }); - const firstId = ask("task-1", "run-1", "first question?"); - const secondId = ask("task-1", "run-1", "second question?"); - resolve("task-1", "run-1", firstId, "answer to the first question"); + it("does not let a stale answer clobber a question asked after a dismiss", () => { + const { dismiss, resolve } = useSideQuestionStore.getState(); + + const firstId = askId("task-1", "run-1", "first question?"); + dismiss("task-1"); + const secondId = askId("task-1", "run-1", "second question?"); + resolve("task-1", "run-1", firstId, "answer to the first"); const entry = useSideQuestionStore.getState().byTaskId["task-1"]; expect(entry?.id).toBe(secondId); @@ -29,9 +70,9 @@ describe("sideQuestionStore stale-answer guard", () => { }); it("does not settle an answer against a run the task has since moved on from", () => { - const { ask, resolve } = useSideQuestionStore.getState(); + const { resolve } = useSideQuestionStore.getState(); - const id = ask("task-1", "run-1", "what changed?"); + const id = askId("task-1", "run-1", "what changed?"); resolve("task-1", "run-2", id, "answer from the old run"); const entry = useSideQuestionStore.getState().byTaskId["task-1"]; diff --git a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts index a308e9763e16..4128cb6e733e 100644 --- a/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts +++ b/products/desktop/packages/ui/src/features/sessions/sideQuestionStore.ts @@ -19,7 +19,8 @@ export type SideQuestionEntry = { interface SideQuestionState { /** Latest side question per task. Ephemeral: never persisted, never part of session history. */ byTaskId: Record; - ask: (taskId: string, taskRunId: string, question: string) => string; + /** Returns null when one is already pending — the card holds a single question. */ + ask: (taskId: string, taskRunId: string, question: string) => string | null; resolve: ( taskId: string, taskRunId: string, @@ -59,9 +60,13 @@ function settle( }; } -export const useSideQuestionStore = create()((set) => ({ +export const useSideQuestionStore = create()((set, get) => ({ byTaskId: {}, ask: (taskId, taskRunId, question) => { + const pending = get().byTaskId[taskId]; + if (pending?.status === "pending" && pending.taskRunId === taskRunId) { + return null; + } const id = crypto.randomUUID(); set((state) => ({ byTaskId: { @@ -91,15 +96,19 @@ export const useSideQuestionStore = create()((set) => ({ * and settle the entry with the answer or error. Fire-and-forget so the * composer clears immediately; the side question runs beside the * conversation, mid-turn or idle. + * + * Returns false when one is already pending, so the caller can say so rather + * than replace a question whose answer is still on its way. */ export function fireSideQuestion( sessionService: Pick, taskId: string, taskRunId: string, question: string, -): void { +): boolean { const { ask, resolve, fail } = useSideQuestionStore.getState(); const id = ask(taskId, taskRunId, question); + if (!id) return false; sessionService .askSideQuestion(taskId, question) .then((answer) => resolve(taskId, taskRunId, id, answer)) @@ -112,4 +121,5 @@ export function fireSideQuestion( ); log.error("Side question failed", error); }); + return true; } From 7ed96241ab09edb0064b874f3e3377dd502b8a78 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:24 -0700 Subject: [PATCH 09/15] feat(agent): answer /btw side questions on cloud sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sandbox runs the same @posthog/agent build as a local session, so the one-shot fork already works there — it just had no way to be asked. The side_question command reaches it over the existing command channel. The answer comes back as the command's result rather than on the event stream, which is what keeps a side question out of the transcript. Cloud runs never negotiate the ACP side-question capability with the desktop client, so the adapter decides instead. A sandbox predating the command rejects it, surfacing as an error on the card. side_question sits behind the same code-access gate as user_message: it drives the agent and spends model tokens on the caller's behalf. --- .../packages/agent/src/server/agent-server.ts | 15 ++++++ .../packages/core/src/cloud-task/schemas.ts | 1 + .../core/src/sessions/sessionService.ts | 34 ++++++++++-- .../sessionServiceSideQuestion.test.ts | 53 ++++++++++++++++--- .../packages/shared/src/sessions.test.ts | 12 +++-- .../desktop/packages/shared/src/sessions.ts | 14 ++--- .../hooks/useSessionCallbacks.test.tsx | 2 +- 7 files changed, 112 insertions(+), 19 deletions(-) diff --git a/products/desktop/packages/agent/src/server/agent-server.ts b/products/desktop/packages/agent/src/server/agent-server.ts index 563934184bad..4911883be4c5 100644 --- a/products/desktop/packages/agent/src/server/agent-server.ts +++ b/products/desktop/packages/agent/src/server/agent-server.ts @@ -1563,6 +1563,21 @@ export class AgentServer { ); } + case POSTHOG_METHODS.SIDE_QUESTION: + case "side_question": { + const question = params.question as string; + + this.logger.debug("Side question requested"); + + // Returned as the command result rather than emitted as a session + // update: a side question is ephemeral, so it must not reach the + // event stream or the persisted transcript. + return await this.session.clientConnection.extMethod( + POSTHOG_METHODS.SIDE_QUESTION, + { sessionId: this.session.acpSessionId, question }, + ); + } + case POSTHOG_NOTIFICATIONS.PERMISSION_RESPONSE: case "permission_response": { const requestId = params.requestId as string; diff --git a/products/desktop/packages/core/src/cloud-task/schemas.ts b/products/desktop/packages/core/src/cloud-task/schemas.ts index 908c213168da..560bb4a3babb 100644 --- a/products/desktop/packages/core/src/cloud-task/schemas.ts +++ b/products/desktop/packages/core/src/cloud-task/schemas.ts @@ -73,6 +73,7 @@ export const sendCommandInput = z.object({ "pi/rpc", "queue_get", "queue_clear", + "side_question", ]), params: z.record(z.string(), z.unknown()).optional(), }); diff --git a/products/desktop/packages/core/src/sessions/sessionService.ts b/products/desktop/packages/core/src/sessions/sessionService.ts index 1acd836cb7ae..75fe944bc219 100644 --- a/products/desktop/packages/core/src/sessions/sessionService.ts +++ b/products/desktop/packages/core/src/sessions/sessionService.ts @@ -62,6 +62,7 @@ import { isTerminalStatus, type Task, } from "@posthog/shared/domain-types"; +import type { SendCommandOutput } from "../cloud-task/schemas"; import type { CommentTarget } from "../comments/anchors"; import type { AgentSessionNotification } from "../notification/agentSessionNotifications"; import { extractPostHogObjectReferences } from "../posthog-objects/references"; @@ -4279,6 +4280,10 @@ export class SessionService { throw new Error("Session is not ready. Try again once it's connected."); } + if (session.isCloud) { + return this.askCloudSideQuestion(session, question); + } + const result = (await this.d.trpc.agent.sideQuestion.mutate({ sessionId: session.taskRunId, question, @@ -4286,6 +4291,29 @@ export class SessionService { return result.answer; } + /** + * Cloud variant: the fork runs in the sandbox, so the question goes over the + * command channel. The answer comes back as the command's result rather than + * on the event stream, which is what keeps it out of the transcript. + */ + private async askCloudSideQuestion( + session: AgentSession, + question: string, + ): Promise { + const result = await this.sendCloudCommand(session, "side_question", { + question, + }); + if (!result.success) { + throw new Error(result.error ?? "Side question failed"); + } + + const answer = (result.result as { answer?: unknown } | undefined)?.answer; + if (typeof answer !== "string" || !answer) { + throw new Error("Side question produced no answer"); + } + return answer; + } + /** * Send the next queued message as its own prompt. * Called internally when a turn completes and there are queued messages. @@ -5476,14 +5504,14 @@ export class SessionService { */ private async sendCloudCommand( session: AgentSession, - method: "permission_response" | "set_config_option", + method: "permission_response" | "set_config_option" | "side_question", params: Record, - ): Promise { + ): Promise { const auth = await this.getCloudCommandAuth(); if (!auth) { throw new Error("No cloud auth credentials available"); } - await this.d.trpc.cloudTask.sendCommand.mutate({ + return await this.d.trpc.cloudTask.sendCommand.mutate({ taskId: session.taskId, runId: session.taskRunId, apiHost: auth.apiHost, diff --git a/products/desktop/packages/core/src/sessions/sessionServiceSideQuestion.test.ts b/products/desktop/packages/core/src/sessions/sessionServiceSideQuestion.test.ts index 8c1ffbf5abb6..3e6b8e9c290a 100644 --- a/products/desktop/packages/core/src/sessions/sessionServiceSideQuestion.test.ts +++ b/products/desktop/packages/core/src/sessions/sessionServiceSideQuestion.test.ts @@ -25,12 +25,19 @@ function makeSession(overrides: Partial = {}): AgentSession { function createHarness(session: AgentSession | undefined, online = true) { const sideQuestionMutate = vi.fn().mockResolvedValue({ answer: "42" }); + const sendCommandMutate = vi + .fn() + .mockResolvedValue({ success: true, result: { answer: "42" } }); const deps = { store: { getSessionByTaskId: () => session, }, log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, getIsOnline: () => online, + fetchAuthState: async () => ({ + cloudRegion: "us", + currentProjectId: 7, + }), trpc: { agent: { sideQuestion: { mutate: sideQuestionMutate }, @@ -38,10 +45,17 @@ function createHarness(session: AgentSession | undefined, online = true) { subscribe: () => ({ unsubscribe: vi.fn() }), }, }, + cloudTask: { + sendCommand: { mutate: sendCommandMutate }, + }, }, } as unknown as SessionServiceDeps; - return { service: new SessionService(deps), sideQuestionMutate }; + return { + service: new SessionService(deps), + sideQuestionMutate, + sendCommandMutate, + }; } describe("SessionService.askSideQuestion", () => { @@ -72,12 +86,9 @@ describe("SessionService.askSideQuestion", () => { ); }); - it.each<[string, Partial]>([ - ["cloud session", { isCloud: true, sideQuestion: true }], - ["codex without the capability", { adapter: "codex" }], - ])("rejects a %s as unsupported", async (_label, overrides) => { + it("rejects a codex session as unsupported", async () => { const { service, sideQuestionMutate } = createHarness( - makeSession(overrides), + makeSession({ adapter: "codex" }), ); await expect(service.askSideQuestion("task-1", "why?")).rejects.toThrow( @@ -86,6 +97,36 @@ describe("SessionService.askSideQuestion", () => { expect(sideQuestionMutate).not.toHaveBeenCalled(); }); + // Cloud sessions aren't in workspace-server's session map, so routing them + // down the local path would fail at runtime. + it("routes a cloud session over the command channel, not the local path", async () => { + const { service, sideQuestionMutate, sendCommandMutate } = createHarness( + makeSession({ isCloud: true }), + ); + + await expect(service.askSideQuestion("task-1", "why?")).resolves.toBe("42"); + expect(sideQuestionMutate).not.toHaveBeenCalled(); + expect(sendCommandMutate).toHaveBeenCalledWith( + expect.objectContaining({ + runId: "run-1", + method: "side_question", + params: { question: "why?" }, + }), + ); + }); + + it.each<[string, Record]>([ + ["the command fails", { success: false, error: "sandbox is gone" }], + ["the sandbox returns no answer", { success: true, result: {} }], + ])("rejects a cloud side question when %s", async (_label, response) => { + const { service, sendCommandMutate } = createHarness( + makeSession({ isCloud: true }), + ); + sendCommandMutate.mockResolvedValue(response); + + await expect(service.askSideQuestion("task-1", "why?")).rejects.toThrow(); + }); + it("rejects when the session is not connected", async () => { const { service, sideQuestionMutate } = createHarness( makeSession({ status: "connecting" }), diff --git a/products/desktop/packages/shared/src/sessions.test.ts b/products/desktop/packages/shared/src/sessions.test.ts index 04fc7a35d38a..ed677e42a4be 100644 --- a/products/desktop/packages/shared/src/sessions.test.ts +++ b/products/desktop/packages/shared/src/sessions.test.ts @@ -143,10 +143,16 @@ describe("sessionSupportsSideQuestion", () => { { isCloud: false, sideQuestion: false, adapter: "claude" }, false, ], - // The fork runs against the local transcript, so cloud never qualifies. + // Cloud runs never negotiate the capability, so the adapter decides: the + // sandbox runs the same agent build and forks its own transcript. [ - "cloud claude with the capability", - { isCloud: true, sideQuestion: true, adapter: "claude" }, + "cloud claude with no capability", + { isCloud: true, sideQuestion: undefined, adapter: "claude" }, + true, + ], + [ + "cloud codex with no capability", + { isCloud: true, sideQuestion: undefined, adapter: "codex" }, false, ], ])("%s", (_label, session, expected) => { diff --git a/products/desktop/packages/shared/src/sessions.ts b/products/desktop/packages/shared/src/sessions.ts index a0f3dc8fcc1f..afe621c23109 100644 --- a/products/desktop/packages/shared/src/sessions.ts +++ b/products/desktop/packages/shared/src/sessions.ts @@ -288,16 +288,18 @@ export function sessionSupportsNativeSteer( /** * Whether the session can answer a one-shot "/btw" side question (a * single-turn, tool-less query forked off the live transcript). Decided by the - * adapter's negotiated `sideQuestion` capability; cloud sessions can't — the - * fork runs against the local transcript. + * adapter's negotiated `sideQuestion` capability where one was negotiated. * - * Fallback: if `sideQuestion` is unset (a session started before capability - * plumbing), local Claude is assumed capable, matching the steer fallback. + * Cloud runs never complete the ACP `initialize` handshake with this client, so + * the capability is inferred from the adapter instead: the sandbox runs the same + * `@posthog/agent` build, and the fork happens there against its own transcript. + * A sandbox predating the `side_question` command rejects it, which surfaces as + * an error on the card rather than a missing command. */ export function sessionSupportsSideQuestion( session: Pick, ): boolean { - if (session.isCloud) return false; if (session.sideQuestion === true) return true; - return session.sideQuestion == null && session.adapter === "claude"; + if (session.sideQuestion === false) return false; + return session.adapter === "claude"; } diff --git a/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.test.tsx b/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.test.tsx index b10d85beb34e..d37841693542 100644 --- a/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.test.tsx +++ b/products/desktop/packages/ui/src/features/sessions/hooks/useSessionCallbacks.test.tsx @@ -323,7 +323,7 @@ describe("useSessionCallbacks.handleSendPrompt askSideQuestion gating", () => { useSessionCallbacks({ taskId: TASK, task, - session: makeSession({ isCloud: true, sideQuestion: true }), + session: makeSession({ sideQuestion: false }), repoPath: "/repo", }), ); From d9c0f76d1a0042e23c5cb08d57407ca5a274bf9d Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:26 -0700 Subject: [PATCH 10/15] fix(agent): register the side_question command schema The agent server validates a command's params against commandParamsSchemas before dispatching it, so a method with no entry there is rejected as "Unknown method" no matter how the dispatch switch is written. The side_question case was unreachable in the sandbox. Found by running a cloud task end to end; the unit tests covered the ACP method and the dispatch case but never the command surface in front of them. --- .../packages/agent/src/server/schemas.test.ts | 18 ++++++++++++++++++ .../packages/agent/src/server/schemas.ts | 9 +++++++++ 2 files changed, 27 insertions(+) diff --git a/products/desktop/packages/agent/src/server/schemas.test.ts b/products/desktop/packages/agent/src/server/schemas.test.ts index a31eef90a1b5..0e65f36d737e 100644 --- a/products/desktop/packages/agent/src/server/schemas.test.ts +++ b/products/desktop/packages/agent/src/server/schemas.test.ts @@ -164,6 +164,24 @@ describe("validateCommandParams", () => { expect(result.success).toBe(false); }); + // A command reaches executeCommand only if it has a schema registered here, + // so a missing entry rejects it as "Unknown method" however the dispatch + // switch is written. + it.each(["side_question", "posthog/side_question"])( + "accepts a valid %s", + (method) => { + const result = validateCommandParams(method, { + question: "what does this repo do?", + }); + + expect(result.success).toBe(true); + }, + ); + + it("rejects side_question without a question", () => { + expect(validateCommandParams("side_question", {}).success).toBe(false); + }); + it("accepts valid permission_response", () => { const result = validateCommandParams("permission_response", { requestId: "abc-123", diff --git a/products/desktop/packages/agent/src/server/schemas.ts b/products/desktop/packages/agent/src/server/schemas.ts index d494d39a83b6..f5ba0c6d7e1c 100644 --- a/products/desktop/packages/agent/src/server/schemas.ts +++ b/products/desktop/packages/agent/src/server/schemas.ts @@ -99,6 +99,13 @@ export const refreshSessionParamsSchema = z.object({ mcpServers: mcpServersSchema, }); +export const sideQuestionParamsSchema = z.object({ + question: z.string().min(1, "question is required"), + // The adapter resolves the ACP session itself; accepted for symmetry with + // the other session-scoped commands. + sessionId: z.string().optional(), +}); + /** * Names of desktop-only local MCP servers designated for relaying into this * run (docs/CLOUD-MCP-RELAY.md). Names only — the sandbox never learns the @@ -143,6 +150,8 @@ export const commandParamsSchemas = { mcp_response: mcpResponseParamsSchema, "posthog/mcp_response": mcpResponseParamsSchema, "_posthog/mcp_response": mcpResponseParamsSchema, + side_question: sideQuestionParamsSchema, + "posthog/side_question": sideQuestionParamsSchema, } as const; export type CommandMethod = keyof typeof commandParamsSchemas; From fb7b145dd88f57d9cc6249fff7c7c6ac02040dec Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:28 -0700 Subject: [PATCH 11/15] fix(agent): isolate the /btw fork from repo-controlled config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one-shot side-question fork inherited the live session's query options, so `mcpServers: {}` only dropped the servers passed in code. The CLI still loaded `.claude/settings*.json` (and the hooks they declare), `.mcp.json`, plugins, and agent frontmatter — repo-controlled code and tools reaching a path documented as tool-less. The fork now sets `settingSources: []`, `plugins: []`, `agents: {}`, and `strictMcpConfig: true`. Generated-By: PostHog Desktop Task-Id: 3e59f50c-bbb2-477e-b212-bfd90b29bd6f --- .../claude/claude-agent.side-question.test.ts | 11 +++++++++++ .../agent/src/adapters/claude/claude-agent.ts | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts index 7856b0606801..74ba6c8457ab 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts @@ -69,6 +69,9 @@ function installFakeSession( mcpServers: { posthog: { type: "http", url: "https://example" }, }, + settingSources: ["user", "project", "local"], + plugins: [{ type: "local", path: "./repo-plugin" }], + agents: { reviewer: { description: "d", prompt: "p" } }, abortController, hooks, }, @@ -140,6 +143,14 @@ describe("ClaudeAcpAgent.extMethod side_question", () => { expect(options.mcpServers).toEqual({}); expect(options.sessionId).toBeUndefined(); expect(options.hooks).toBeUndefined(); + + // Nothing the repo controls runs for a side question: no filesystem + // settings (and so no hooks they declare), no plugins, no agents, and no + // MCP servers merged back in from `.mcp.json` or agent frontmatter. + expect(options.settingSources).toEqual([]); + expect(options.plugins).toEqual([]); + expect(options.agents).toEqual({}); + expect(options.strictMcpConfig).toBe(true); // Fresh controller: aborting the one-shot must not poison the session. expect(options.abortController).not.toBe(abortController); diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts index 79b7d996741f..eeea9102fba2 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts @@ -2016,6 +2016,17 @@ export class ClaudeAcpAgent extends BaseAcpAgent { // Never reuse in-process MCP instances ("Already connected to a // transport"); the fork has no tools, so it needs no servers. mcpServers: {}, + // `mcpServers: {}` alone only drops the servers passed in code — the + // CLI would still merge `.mcp.json`, user settings, and plugin/agent + // frontmatter servers back in. + strictMcpConfig: true, + // A side question is an isolated read of the transcript, so nothing + // the repo can write should get to run for it. `settingSources: []` + // keeps `.claude/settings*.json` (and the hooks they declare) off the + // fork, and plugins and agents ship their own hooks and skills. + settingSources: [], + plugins: [], + agents: {}, abortController, // `rest.model` is the creation-time value; the user may have // switched models since, so answer on the live session model. From bd43edd5144008ff8a2f6cc7915f5bf97e74e7d4 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:30 -0700 Subject: [PATCH 12/15] fix(agent): resume the current SDK session for side questions after /clear The /btw fork resumed `this.sessionId`, the stable ACP id. `/clear` swaps `session.sdkSessionId` to a fresh session and deletes the pre-clear transcript, so after any clear the fork resumed a retired/deleted transcript and the query failed. Resume `this.session.sdkSessionId` instead, matching the sibling resume path in refreshSession. Adds a regression test asserting the fork targets the current SDK session id, not the ACP id, once the two diverge. Generated-By: PostHog Desktop Task-Id: 77880d7a-4e6f-4e27-88e8-1cc58ec9478e --- .../claude/claude-agent.side-question.test.ts | 21 ++++++++++++++++++- .../agent/src/adapters/claude/claude-agent.ts | 5 ++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts index 74ba6c8457ab..128d42909ec3 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts @@ -53,7 +53,7 @@ function makeAgent(): Agent { function installFakeSession( agent: Agent, sessionId: string, - overrides: Partial<{ modelId: string }> = {}, + overrides: Partial<{ modelId: string; sdkSessionId: string }> = {}, ) { const abortController = new AbortController(); const input = new Pushable(); @@ -62,6 +62,9 @@ function installFakeSession( const session = { query: oldQuery, + // At creation the SDK id matches the ACP id; `/clear` later swaps only + // this one, leaving `agent.sessionId` stable. + sdkSessionId: overrides.sdkSessionId ?? sessionId, queryOptions: { sessionId, cwd: "/tmp/repo", @@ -172,6 +175,22 @@ describe("ClaudeAcpAgent.extMethod side_question", () => { expect((agent as unknown as { sessionId: string }).sessionId).toBe("s-2"); }); + it("forks the current SDK session id, not the stable ACP id, after /clear swaps it", async () => { + const agent = makeAgent(); + // /clear keeps the ACP id stable but points sdkSessionId at the fresh + // session and deletes the pre-clear transcript. The fork must resume the + // fresh id, or it resumes a retired/deleted transcript. + installFakeSession(agent, "acp-1", { sdkSessionId: "sdk-after-clear" }); + nextQueryMessages = [ + assistantText("ok"), + { type: "result", subtype: "success" }, + ]; + + await agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { question: "hm?" }); + + expect(lastQueryCall.options?.resume).toBe("sdk-after-clear"); + }); + it("answers on the live session model, not the creation-time option", async () => { const agent = makeAgent(); installFakeSession(agent, "s-3", { modelId: "claude-opus-4-8" }); diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts index eeea9102fba2..c9d1903ba6fe 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts @@ -2001,7 +2001,10 @@ export class ClaudeAcpAgent extends BaseAcpAgent { } = this.session.queryOptions; const options: Options = { ...rest, - resume: this.sessionId, + // Fork the current SDK session, not the stable ACP id. `/clear` swaps + // `sdkSessionId` to a fresh session and deletes the pre-clear + // transcript, so `this.sessionId` would resume a retired one. + resume: this.session.sdkSessionId, forkSession: true, maxTurns: 1, // Belt and braces: remove the toolset entirely and deny anything From 5fecc33c511ff879958727ee1936938e6a81e229 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:32 -0700 Subject: [PATCH 13/15] fix(agent): abort in-flight side question when the session closes The /btw fork owns a separate AbortController that the base close path never sees, so closing a session mid-answer left the fork streaming (and consuming tokens) until its 120s timeout fired. closeSession now aborts sideQuestionAbort before delegating to the base close path, matching the supersede path that already aborts the fork. Adds a regression test that starts a gated side question, closes the session, and asserts the fork's controller was aborted. Generated-By: PostHog Desktop Task-Id: 13761e40-4243-459f-86d3-cbadf3174f28 --- .../claude/claude-agent.side-question.test.ts | 29 +++++++++++++++++++ .../agent/src/adapters/claude/claude-agent.ts | 5 ++++ 2 files changed, 34 insertions(+) diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts index 128d42909ec3..601dec81b241 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts @@ -263,6 +263,35 @@ describe("ClaudeAcpAgent.extMethod side_question", () => { await first.catch(() => {}); }); + it("aborts an in-flight side question when the session closes", async () => { + const agent = makeAgent(); + installFakeSession(agent, "s-8"); + + let release: () => void = () => {}; + nextQueryGate = new Promise((resolve) => { + release = resolve; + }); + nextQueryMessages = [ + assistantText("answer"), + { type: "result", subtype: "success" }, + ]; + + const inflight = agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { + question: "still going?", + }); + await vi.waitFor(() => + expect(lastQueryCall.options?.abortController).toBeDefined(), + ); + const forkAbort = lastQueryCall.options?.abortController as AbortController; + + await agent.closeSession(); + + expect(forkAbort.signal.aborted).toBe(true); + + release(); + await inflight.catch(() => {}); + }); + it("wraps SDK failures (e.g. nothing to resume) in a clear RequestError", async () => { const agent = makeAgent(); installFakeSession(agent, "s-7"); diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts index c9d1903ba6fe..c1f685204e6d 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts @@ -378,6 +378,11 @@ export class ClaudeAcpAgent extends BaseAcpAgent { override async closeSession(): Promise { try { + // A /btw fork runs on its own controller that the base close path never + // touches, so without this an in-flight side question keeps streaming + // (and burning tokens) until its own timeout fires after the session is + // gone. + this.sideQuestionAbort?.abort(); await super.closeSession(); } finally { this.enrichment?.dispose(); From c0a22fec8795cd539c0be1183e7098ac3ca0ae2e Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:34 -0700 Subject: [PATCH 14/15] fix(agent): drop inherited outputFormat from side question fork A structured task run stores a json_schema outputFormat on the live session's query options. The /btw fork spread `...rest` over those options and reset every other inherited key (tools, mcpServers, plugins, etc.) but not outputFormat, so on a structured session the side question answer was forced into the task's unrelated schema and rendered as schema-shaped JSON or dropped as no answer. Strip outputFormat in the destructure alongside sessionId and hooks so the fork always answers as plain text. Adds a regression test that gives the live session a json_schema outputFormat and asserts the fork's options carry none. Generated-By: PostHog Desktop Task-Id: 13761e40-4243-459f-86d3-cbadf3174f28 --- .../claude/claude-agent.side-question.test.ts | 24 +++++++++++++++++++ .../agent/src/adapters/claude/claude-agent.ts | 7 ++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts index 601dec81b241..3ac77abbf7ef 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.side-question.test.ts @@ -263,6 +263,30 @@ describe("ClaudeAcpAgent.extMethod side_question", () => { await first.catch(() => {}); }); + it("strips the live session's outputFormat so the answer stays plain text", async () => { + const agent = makeAgent(); + const { session } = installFakeSession(agent, "s-struct"); + // A structured task run stores a json_schema outputFormat on the live + // session; inheriting it forces the answer into that unrelated shape. + ( + session as unknown as { queryOptions: Record } + ).queryOptions.outputFormat = { + type: "json_schema", + schema: { type: "object" }, + }; + nextQueryMessages = [ + assistantText("plain answer"), + { type: "result", subtype: "success" }, + ]; + + const result = await agent.extMethod(POSTHOG_METHODS.SIDE_QUESTION, { + question: "what changed?", + }); + + expect(result).toEqual({ answer: "plain answer" }); + expect(lastQueryCall.options?.outputFormat).toBeUndefined(); + }); + it("aborts an in-flight side question when the session closes", async () => { const agent = makeAgent(); installFakeSession(agent, "s-8"); diff --git a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts index c1f685204e6d..74d3cb10a65c 100644 --- a/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts +++ b/products/desktop/packages/agent/src/adapters/claude/claude-agent.ts @@ -1997,11 +1997,14 @@ export class ClaudeAcpAgent extends BaseAcpAgent { const abortController = new AbortController(); this.sideQuestionAbort = abortController; try { - // Drop `sessionId` (identity comes from `resume`) and `hooks` (they - // close over live-session caches and task state). + // Drop `sessionId` (identity comes from `resume`), `hooks` (they close + // over live-session caches and task state), and `outputFormat` (a + // structured task run stores a json_schema here that would force the + // plain-text answer into the task's unrelated shape). const { sessionId: _sessionId, hooks: _hooks, + outputFormat: _outputFormat, ...rest } = this.session.queryOptions; const options: Options = { From ac26fe3b62cbb109ff49880c6d42c95ece781d64 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Thu, 20 Aug 2026 13:17:36 -0700 Subject: [PATCH 15/15] fix(agent): hide /btw from the command menu on unsupported sessions The command menu merged /btw into every session's suggestions, so on a session that cannot answer side questions (for example a codex session, or the new-task composer with no session) picking /btw dead-ended in an error toast. getCommandSuggestions now filters /btw out unless sessionSupportsSideQuestion is true, reusing the same capability check the runtime callback uses. The command's own runtime guard stays for users who type /btw by hand. Adds parameterized cases asserting /btw shows on a claude session and is hidden on a codex session and when there is no session. Generated-By: PostHog Desktop Task-Id: 13761e40-4243-459f-86d3-cbadf3174f28 --- .../src/features/message-editor/commands.ts | 4 ++- .../suggestions/getSuggestions.test.ts | 20 ++++++++++++++ .../suggestions/getSuggestions.ts | 27 +++++++++++++++---- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/products/desktop/packages/ui/src/features/message-editor/commands.ts b/products/desktop/packages/ui/src/features/message-editor/commands.ts index c7a63bc0217d..c665cfab45e8 100644 --- a/products/desktop/packages/ui/src/features/message-editor/commands.ts +++ b/products/desktop/packages/ui/src/features/message-editor/commands.ts @@ -107,8 +107,10 @@ const addDirCommand: CodeCommand = { }, }; +export const BTW_COMMAND_NAME = "btw"; + const btwCommand: CodeCommand = { - name: "btw", + name: BTW_COMMAND_NAME, description: "Ask a quick side question without interrupting the conversation", input: { hint: "your question" }, diff --git a/products/desktop/packages/ui/src/features/message-editor/suggestions/getSuggestions.test.ts b/products/desktop/packages/ui/src/features/message-editor/suggestions/getSuggestions.test.ts index 0d0a7181ebf7..e91726432dc1 100644 --- a/products/desktop/packages/ui/src/features/message-editor/suggestions/getSuggestions.test.ts +++ b/products/desktop/packages/ui/src/features/message-editor/suggestions/getSuggestions.test.ts @@ -154,6 +154,26 @@ const SCENARIOS: Scenario[] = [ sessionCommands: [{ name: "agent-cmd", description: "From agent" }], expectContains: ["agent-cmd", "fallback-skill"], }, + { + name: "claude session offers /btw", + contextTaskId: TASK_ID, + adapter: "claude", + sessionCommands: [], + expectContains: ["btw"], + }, + { + name: "codex session hides /btw (side questions unsupported)", + contextTaskId: TASK_ID, + adapter: "codex", + sessionCommands: [], + expectContains: ["good"], + expectNotContains: ["btw"], + }, + { + name: "new-task composer with no session hides /btw", + expectContains: ["good"], + expectNotContains: ["btw"], + }, ]; describe("getCommandSuggestions", () => { diff --git a/products/desktop/packages/ui/src/features/message-editor/suggestions/getSuggestions.ts b/products/desktop/packages/ui/src/features/message-editor/suggestions/getSuggestions.ts index 8274a0ecdebc..798fb63879af 100644 --- a/products/desktop/packages/ui/src/features/message-editor/suggestions/getSuggestions.ts +++ b/products/desktop/packages/ui/src/features/message-editor/suggestions/getSuggestions.ts @@ -9,12 +9,13 @@ import { shapeCommandSuggestions, shapeFileSuggestions, } from "@posthog/core/message-editor/suggestions"; +import { sessionSupportsSideQuestion } from "@posthog/shared"; import { getAvailableCommandsForTask, useSessionStore, } from "@posthog/ui/features/sessions/sessionStore"; import { fetchRepoFiles, searchFiles } from "../../repo-files/useRepoFiles"; -import { CODE_COMMANDS } from "../commands"; +import { BTW_COMMAND_NAME, CODE_COMMANDS } from "../commands"; import { useDraftStore } from "../draftStore"; import { searchGithubRefs } from "../hostApi"; import type { @@ -25,13 +26,19 @@ import type { function getTaskCommandContext(taskId: string | undefined): { adapter: string | undefined; + supportsSideQuestion: boolean; commands: ReturnType; } { - if (!taskId) return { adapter: undefined, commands: null }; + if (!taskId) + return { adapter: undefined, supportsSideQuestion: false, commands: null }; const state = useSessionStore.getState(); const taskRunId = state.taskIdIndex[taskId]; + const session = taskRunId ? state.sessions[taskRunId] : undefined; return { - adapter: taskRunId ? state.sessions[taskRunId]?.adapter : undefined, + adapter: session?.adapter, + supportsSideQuestion: session + ? sessionSupportsSideQuestion(session) + : false, commands: getAvailableCommandsForTask(taskId), }; } @@ -104,7 +111,11 @@ export function getCommandSuggestions( // commands, so keep merging the trpc-fetched skills fallback for GPT tasks. // `null` means "agent hasn't reported yet"; an empty array means "agent // reported empty". - const { adapter, commands: sessionCommands } = getTaskCommandContext(taskId); + const { + adapter, + supportsSideQuestion, + commands: sessionCommands, + } = getTaskCommandContext(taskId); const draftCommands = store.commands[sessionId] ?? []; const localDraftCommands = draftCommands.filter((cmd) => cmd.localSkill); const agentCommands = @@ -113,7 +124,13 @@ export function getCommandSuggestions( : adapter === "codex" ? mergeCommands(sessionCommands, draftCommands) : [...sessionCommands, ...localDraftCommands]; - const commands = mergeCommands(CODE_COMMANDS, agentCommands); + // Only offer /btw where the session can actually answer a side question; + // otherwise picking it from the menu dead-ends in an error toast. The + // runtime guard in the command still stands for users who type /btw by hand. + const codeCommands = supportsSideQuestion + ? CODE_COMMANDS + : CODE_COMMANDS.filter((cmd) => cmd.name !== BTW_COMMAND_NAME); + const commands = mergeCommands(codeCommands, agentCommands); const filtered = searchCommands(commands, query); return shapeCommandSuggestions(filtered);