feat(desktop): /clear a finished cloud run without booting a sandbox - #82950
Conversation
|
😎 Merged successfully - details. |
|
React Doctor found 9 issues in 4 files · 9 warnings. 9 warnings
Reviewed by React Doctor for commit |
HostHog preview —
|
b237ecd to
12252ee
Compare
🦔 ReviewHog reviewed this pull requestFound 0 must fix, 0 should fix, 1 consider. Published 1 finding (view the review). |
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
12252ee to
7648ebc
Compare
Prompt To Fix All With AI### Issue 1
products/desktop/packages/shared/src/slash-commands.ts:14-18
**Use the standard absence sentinel**
`leadingSlashCommand` introduces `undefined` as an intentional absent result and codifies it across the shared API, callers, and tests. Use `null` for the no-command result to follow the repository convention and avoid a broader cleanup later.
### Issue 2
products/desktop/packages/core/src/sessions/sessionServiceCloudClear.test.ts:76-111
**Preserve dependency mock type safety**
The new harness double-casts an incomplete dependency object through `unknown`, preventing TypeScript from checking it against `SessionServiceDeps`. This allows required dependency changes to produce runtime-only test failures instead of identifying an incomplete mock during type checking.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(desktop): /clear a finished cloud r..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Routes /clear on terminal (finished) cloud runs in the PostHog Desktop app through the backend “clear conversation boundary” endpoint, avoiding the cost of resuming/booting a sandbox just to clear a conversation that will be rebuilt from logs anyway.
Changes:
- Added a shared
leadingSlashCommandhelper so the desktop client and agent adapter use the same “leading/commandtoken” detection. - Updated
SessionServiceto call the cloudclear_conversationendpoint (when the run is terminal and the capability is present) and repaint the thread from persisted logs, with a small retry budget for S3-read lag. - Added targeted tests for the finished-cloud-run
/clearpath, including retry/fallback behavior and dedupe on later resume hydration.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| products/desktop/packages/shared/src/slash-commands.ts | Introduces shared leading slash-command tokenizer for dispatch parity. |
| products/desktop/packages/shared/src/slash-commands.test.ts | Unit tests for the shared slash-command tokenizer behavior. |
| products/desktop/packages/shared/src/index.ts | Re-exports leadingSlashCommand from the shared package. |
| products/desktop/packages/core/src/sessions/sessionServiceCloudClear.test.ts | Adds a focused test suite covering /clear on finished cloud runs (endpoint call, repaint, retry, fallback, dedupe). |
| products/desktop/packages/core/src/sessions/sessionService.ts | Adds terminal-run /clear routing to clear_conversation + repaint logic, with retry/fallback. |
| products/desktop/packages/core/src/sessions/sessionEvents.ts | Adds local fallback event creation for “conversation cleared” frames and documents log promotion behavior. |
| products/desktop/packages/api-client/src/posthog-client.ts | Adds clearTaskRunConversation API client method for the new backend endpoint. |
| products/desktop/packages/agent/src/adapters/claude/claude-agent.ts | Switches adapter command detection to use shared leadingSlashCommand for parity. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The session service routes a /clear on a finished cloud run to POST
runs/{id}/clear_conversation instead of resuming into a sandbox. After
the call succeeds, it rehydrates the session from the updated run log,
so the painted /clear message and the cleared divider carry the
backend's persisted timestamps; that lets resume-time reconciliation
fold them into a single copy instead of rendering the pair twice. A
locally stamped fallback pair covers the rare case where the log fetch
cannot confirm the boundary landed.
The path is gated on the agent's conversationClear capability from the
run log. An older agent ignores the marker and resumes the
conversation itself, so an ordinary resume is the honest degradation
when the capability is absent.
A shared tokenizer (leadingSlashCommand in @posthog/shared) keeps the
desktop client's /clear detection identical to the agent adapter's,
and clear_conversation errors surface through the existing
extractRequestErrorMessage helper.
Generated-By: PostHog Desktop
Task-Id: 2578f561-b94e-46ac-8546-d3368f5098bb
7648ebc to
5efd513
Compare
|
/trunk merge |
🤖 CI report✅ Trunk lane — does not run backend Python testsThis PR is assigned to a lane that does not run backend Python tests. |
extractRequestErrorMessage passed through any non-empty error/detail
string from the backend, including Django's generic {"detail":"Not
found."} for an unmatched route. On a pre-#76943 backend without the
clear_conversation endpoint, that surfaced /clear's failure as a bare
"Not found." with no indication a clear was attempted or what to do
next. Treat that literal DRF placeholder as non-actionable and fall
through to the endpoint's own fallback message plus status code.
Generated-By: PostHog Desktop
Task-Id: 2578f561-b94e-46ac-8546-d3368f5098bb
hydrateCloudTaskSessionFromLogs probes getTaskRunSessionLogsPage before falling back to getTaskRunSessionLogsResult. The harness only stubbed the latter, so the probe threw, the hydration bailed early, and the repaint assertions never saw a call. Generated-By: PostHog Desktop Task-Id: 2578f561-b94e-46ac-8546-d3368f5098bb
Problem
Typing
/clearon a finished cloud run in the desktop app boots a whole sandbox: resume the run, rebuild the conversation, clear it, idle. A full run's cost to clear a conversation the next run rebuilds from the log anyway.#76943 added the backend boundary endpoint and the web composer support. The desktop app still pays the sandbox.
Changes
/clearon a finished cloud run toPOST runs/{id}/clear_conversationinstead of resuming./clearand divider twice./clearon an already-cleared run does nothing and sends nothing.conversationClearcapability from the run log. An older agent gets today's behavior: an ordinary resume, and the agent performs the clear itself.Note
Depends on #76943's endpoint, now live (verified in US production, 2026-08-14). Against an older backend,
/clearon a finished run surfaces "Couldn't clear the conversation." and paints nothing.No visible change outside the
/clearflow.How did you test this code?
sessionServiceCloudClear.test.ts(new, 8 tests) pins the contract:/clearshort-circuits with no second POST./clear, fall through to an ordinary resume./clearon a finished run painted the message and divider with no sandbox boot and no new run./clearchanged nothing.Automatic notifications
Docs update
None. No user-facing docs cover
/clearyet.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Split out of #76943 by Claude Code (PostHog Desktop session) after the "Desktop backend coupling" check required separating desktop from backend changes; the commit content matches what charlesvien approved there. The duplicate-pair fix, review follow-ups, and the CDP-driven manual test were agent-executed with human approval at each decision. Skills invoked across the sessions: /stacking-prs, /writing-tests, /writing-code-comments, /writing-pr-descriptions, /resolve-conflicts, /address-pr-reviews, /simplify, /review-code, /squash, test-electron-app.
Created with PostHog Desktop