feat(agent): implement /clear command#3546
Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
f58e90b to
30f05dc
Compare
Intercept /clear in the Claude adapter instead of forwarding it to the SDK: retire the current query and swap in a brand-new SDK session (fresh id, no resume) under the same ACP session. Persist a _posthog/conversation_cleared marker plus an updated sdk_session mapping to the append-only session log, and make the rehydration paths (jsonl hydration, ResumeSaga) treat the marker as a conversation boundary so desktop reconnects and cloud resumes rebuild only the post-clear conversation. The UI renders a "Conversation cleared" divider and resets the context indicator. Generated-By: PostHog Code Task-Id: 7f180c25-f99b-4c96-ac84-f39c0e887a37
- Parallelize post-clear notifications instead of sequential awaits - Use isNotification() helper instead of set-based method checking - Remove unused variables and redundant state resets - Improve comments for session id handling
Only broadcast (and thus log) the "/clear" prompt once the new session is confirmed live, so a timeout leaves no orphaned entry in the log. Add a "Clearing…" status indicator, mirroring the existing compaction spinner, so a slow or failed clear gives the user feedback instead of going silent. Delete the local session file left under the original session id after a clear so a cold reconnect can't mistake it for current and resume the pre-clear conversation.
e0d3e15 to
1c5c6ae
Compare
🦔 ReviewHog reviewed this pull requestFound 0 must fix, 3 should fix, 0 consider. Published 3 findings (view the review). |
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
There was a problem hiding this comment.
ReviewHog Report
Feature
Issues: 2 issues
Files (9)
packages/agent/src/acp-extensions.tspackages/core/src/sessions/acpNotifications.tspackages/agent/src/adapters/base-acp-agent.tspackages/agent/src/adapters/claude/types.tspackages/agent/src/adapters/claude/claude-agent.tspackages/agent/src/adapters/claude/session/commands.tspackages/agent/src/adapters/claude/session/jsonl-hydration.tspackages/agent/src/sagas/resume-saga.tspackages/agent/src/adapters/claude/UPSTREAM.md
What were the main changes
- New
_posthog/conversation_clearednotification constant marks the /clear boundary in the append-only session log (agent + core copies) base-acp-agent.hasSessionextracted as an overridable hook so cancel/reconnect can match either the ACP session id or a post-clear SDK session id- Claude adapter intercepts
/clearinprompt(): guards against busy/queued turns, retires the current SDK query via newretireQuery(extracted fromrefreshSession), and swaps in a fresh SDK session under the same ACP session id, tracked via newSession.sdkSessionId clearConversationbroadcasts a 'clearing' status, only persists the /clear prompt once the new session is confirmed live, emits the conversation_cleared marker + sdk_session mapping + zeroed usage_update, deletes the stale local jsonl, and handles timeout with a 'clearing_failed' status and no orphaned log entry/clearre-added to advertised slash commands (adapter-implemented, not forwarded to SDK) viagetAvailableSlashCommandsrebuildConversation(jsonl hydration) andResumeSaga.findSessionId/rebuildConversationtreat CONVERSATION_CLEARED as a boundary so cloud/desktop resume rehydrate only post-clear turns using the fresh SDK session id- UPSTREAM.md updated to document that /clear is now reimplemented locally, superseding the prior 'hide /clear' note
Frontend
Issues: 1 issue
Files (4)
packages/ui/src/features/sessions/components/buildConversationItems.tspackages/ui/src/features/sessions/components/session-update/ConversationClearedView.tsxpackages/ui/src/features/sessions/components/session-update/SessionUpdateView.tsxpackages/ui/src/features/sessions/components/session-update/StatusNotificationView.tsx
What were the main changes
buildConversationItemsrenders a 'conversation_cleared' item on the CONVERSATION_CLEARED notification and handles the new 'clearing'/'clearing_failed' status rows (mirroring compaction)- New
ConversationClearedViewrenders a 'Conversation cleared' divider (chat-thread separator or legacy bordered row) SessionUpdateViewwires the new 'conversation_cleared' item type to the new view componentStatusNotificationView/CompactingStatusViewgeneralized to show a 'Clearing conversation…' spinner and a 'clearing_failed' outcome row, reusing the compaction spinner pattern with a configurable label
Other findings (outside the changed lines)
Valid issues on this PR's files that sit on lines GitHub won't let us comment on inline.
Missing isClearing state leaves the generic "Generating…" spinner running alongside the dedicated "Clearing…" row
Priority: should_fix | File: packages/ui/src/features/sessions/components/buildConversationItems.ts:748-781 | Category: bug
Why we think it's a valid issue
- Checked:
handleRuntimeStatus/markRuntimeStatusCompletein buildConversationItems.ts (748-793, 873-894), theisCompactingthread through useConversationItems.ts → ConversationView.tsx:158/468 → SessionFooter.tsx, the SessionFooter gate, StatusNotificationView.tsx clearing render, the adapter's clearConversation (claude-agent.ts:1517-1673) and prompt() dispatch (line 475), and isPromptPending lifecycle in sessionService.ts. - Found:
b.isCompactingis set only in thecompactingbranch (buildConversationItems.ts:753) and reset only for"compacting"(line 875); theclearing/clearing_failedbranches (765-780) set no analogous flag.BuildResult.isCompactingis the only busy flag threaded toSessionFooter, whose sole gate isif (isPromptPending && !isCompacting)(SessionFooter.tsx:52) — noisClearingprop exists (props at 14-28). - Found:
/clearruns through the ordinary prompt RPC: prompt() returnsclearConversation(params)(claude-agent.ts:475-478), andsendLocalPromptsetsisPromptPending: true(sessionService.ts:2673) flipping it false only afteragent.prompt.mutateresolves (2708-2711) — i.e. after the whole clear. MeanwhileclearConversationemitsclearing(isComplete omitted → false) at line 1544-1547 before the swap, and StatusNotificationView.tsx:170-179 renders that as aCompactingStatusView label="Clearing conversation…"spinner row. - Impact: For the entire duration of every
/clear, both the generic footer "Generating…" indicator + spinningSlotMachineLeverand the dedicated "Clearing conversation…" row render simultaneously — 100% reproducible, not an edge case. On a slow clear this persists up toSESSION_VALIDATION_TIMEOUT_MS = 30_000ms (claude-agent.ts:153), the exact scenario the clearing spinner was added for, where the footer misleadingly says the agent is "Generating…". This directly contradicts the PR's stated intent that clearing "mirrors the existing compaction spinner" (compaction suppresses the generic indicator via the!isCompactinggate; clearing does not). The suggested fix (addisClearing, thread it, extend the gate to!isCompacting && !isClearing) is the correct mirror of the compaction path.
Issue description
The new clearing/clearing_failed handling in handleRuntimeStatus mirrors the compacting branches (both push a dedicated status row and complete it via markRuntimeStatusComplete), but it never sets an isClearing-equivalent of b.isCompacting. BuildResult/ItemBuilder expose isCompacting (set true at line 753, reset via markRuntimeStatusComplete(b, "compacting") at line 875) but there is no analogous flag for clearing. Downstream, SessionFooter.tsx:52 uses if (isPromptPending && !isCompacting) to suppress the generic "Generating…" spinner + SlotMachineLever while compaction's dedicated row is shown. Since /clear is dispatched as an ordinary session/prompt RPC, isPromptPending stays true for the whole clear (it only flips false when the RPC resolves after the entire clear completes), while isCompacting stays false throughout (it's never set for clearing). Verified by tracing useConversationItems → ConversationView.tsx:468 / ChatThreadFooter.tsx:62, which pass isCompacting straight from BuildResult.isCompacting with nothing else merged in, and confirming SessionFooter has no isClearing/isBusy prop. Consequently, for the entire duration of every /clear, the generic footer spinner/lever and the dedicated "Clearing conversation…" status row (StatusNotificationView.tsx:170-180) render simultaneously — contradicting the PR's stated intent that the clearing indicator "mirrors the existing compaction spinner" (compaction correctly suppresses the generic indicator; clearing does not).
Suggested fix
Add an isClearing field to ItemBuilder/BuildResult (set true when status.status === "clearing" and not complete, reset to false in markRuntimeStatusComplete when completing a "clearing" row, same as the existing isCompacting handling), thread it through useConversationItems, ConversationView.tsx, and ChatThreadFooter.tsx, and extend SessionFooter.tsx's gate to if (isPromptPending && !isCompacting && !isClearing) so the generic spinner is suppressed during a clear just like it is during compaction.
| await this.client.extNotification(POSTHOG_NOTIFICATIONS.STATUS, { | ||
| sessionId: params.sessionId, | ||
| status: "clearing", | ||
| }); |
There was a problem hiding this comment.
clearConversation() has no error/rollback path other than the timeout branch, so any other failure leaves the client spinner stuck and the session silently broken
Why we think it's a valid issue
- Checked:
clearConversation(claude-agent.ts:1544-1697),retireQuery(1480-1505),withTimeout(utils/common.ts:8-20), the consumer wiring (ensureConsumer535-548), and the UI spinner lifecycle (buildConversationItems.ts:745-781, StatusNotificationView.tsx). - Found:
withTimeoutonly converts the timeout race outcome —operationPromise = operation.then(...)(common.ts:15-19), so a genuine rejection of the wrapped promise propagates. Both awaitedwithTimeoutcalls in the clear path can therefore reject non-timeout:retireQuery'sawait withTimeout(oldConsumer, 5000)(1503) — and the consumer genuinely can reject (the codebase attaches a.catchlogging "Consumer terminated unexpectedly" at 542) — andawait withTimeout(newQuery.initializationResult(), …)(1584) on SDK subprocess crash / a concurrent abort of the livenewAbortController(assigned to the session at 1582). - Found: Only the
result.result === "timeout"branch (1588-1604) recovers (terminateQuery +queryClosed = true+clearing_failed). There is no surrounding try/catch, andsession.query/input/queryOptions/abortControllerare overwritten at 1579-1582 before the init await at 1584 — so a non-timeout rejection leaves the session pointing at a half-initialized query withqueryClosedunset (so the guards at 1521/485 don't fire), and the"clearing"STATUS emitted at 1544-1547 is never resolved. - Found: The UI confirms the stuck-spinner harm: the in-progress
clearingrow is flipped complete only by aclearingstatus withisComplete(buildConversationItems.ts:765-769) or aclearing_failed(771-780); nothing else (turn-end, error) clears it, so a non-timeout failure leaves "Clearing…" spinning permanently until a reload. - Impact: A reachable failure mode (SDK init reject / consumer reject) leaves a permanently stuck spinner plus a half-swapped, non-
queryClosedsession that accepts further prompts into a dead query — directly defeating the PR's stated "failed clear gives the user feedback" hardening goal. Not defensive paranoia: these rejections are real (the code's own.catchacknowledges consumer rejection), and the author already handles the sibling timeout case identically, so extending it is completing intended behavior. - Priority: Lowered must_fix → should_fix: the defect only triggers on an abnormal non-timeout failure during the clear swap (the success and timeout paths are both handled), so it degrades an already-failing operation rather than breaking the happy path; real and worth fixing, but not happy-path/data-loss severity.
Issue description
clearConversation() broadcasts a "clearing" STATUS notification (1544-1547), then does a sequence of operations that can each fail for reasons other than a timeout: retireQuery(session) (1549) awaits withTimeout(oldConsumer, 5000) which does NOT catch a genuine rejection of oldConsumer (only converts the timeout race outcome, per withTimeout's implementation in packages/agent/src/utils/common.ts — a rejection of the wrapped promise still propagates as a rejection); session.buildInProcessMcpServers() (1561) and query({...}) (1577) can throw synchronously (e.g. createLocalToolsMcpServer/resolveGithubToken failures); and newQuery.initializationResult() (1584-1587) can reject for reasons other than timing out (e.g. the SDK subprocess crashing, or the newAbortController being aborted by a concurrent cancel()/interrupt() — see the related concurrency finding). Only the explicit result.result === "timeout" branch (1588-1604) cleans up: it calls terminateQuery, sets session.queryClosed = true, and emits a clearing_failed STATUS notification. Any other exception in this method propagates uncaught: the client's "Clearing…" spinner (from the notification at 1544-1547) is never resolved with either isComplete: true or clearing_failed, so it hangs forever from the user's perspective, and session.query/session.input/session.abortController have already been overwritten with the broken/half-initialized new Query (1579-1582) without session.queryClosed being set, so the session is left in a corrupted state that isn't gated against further prompts the way the timeout path (and closeQueryStream) intends.
Suggested fix
Wrap the risky portion of clearConversation (from retireQuery through the notifications) in a try/catch (or try/finally) that, on ANY error — not just the timeout case — mirrors the timeout branch: call terminateQuery on the new query, set session.queryClosed = true (or otherwise restore a known-good state), and emit a clearing_failed STATUS notification before rethrowing. This guarantees the client never sees an unresolved "clearing" spinner and the session is never left half-swapped.
Prompt to fix with AI (copy-paste)
## Context
@packages/agent/src/adapters/claude/claude-agent.ts#L1544-1547
@packages/agent/src/adapters/claude/claude-agent.ts#L1576-1587
@packages/agent/src/adapters/claude/claude-agent.ts#L1588-1604
<issue_description>
clearConversation() broadcasts a `"clearing"` STATUS notification (1544-1547), then does a sequence of operations that can each fail for reasons other than a timeout: `retireQuery(session)` (1549) awaits `withTimeout(oldConsumer, 5000)` which does NOT catch a genuine rejection of `oldConsumer` (only converts the *timeout* race outcome, per `withTimeout`'s implementation in `packages/agent/src/utils/common.ts` — a rejection of the wrapped promise still propagates as a rejection); `session.buildInProcessMcpServers()` (1561) and `query({...})` (1577) can throw synchronously (e.g. `createLocalToolsMcpServer`/`resolveGithubToken` failures); and `newQuery.initializationResult()` (1584-1587) can reject for reasons other than timing out (e.g. the SDK subprocess crashing, or the `newAbortController` being aborted by a concurrent `cancel()`/`interrupt()` — see the related concurrency finding). Only the explicit `result.result === "timeout"` branch (1588-1604) cleans up: it calls `terminateQuery`, sets `session.queryClosed = true`, and emits a `clearing_failed` STATUS notification. Any other exception in this method propagates uncaught: the client's "Clearing…" spinner (from the notification at 1544-1547) is never resolved with either `isComplete: true` or `clearing_failed`, so it hangs forever from the user's perspective, and `session.query`/`session.input`/`session.abortController` have already been overwritten with the broken/half-initialized new Query (1579-1582) without `session.queryClosed` being set, so the session is left in a corrupted state that isn't gated against further prompts the way the timeout path (and `closeQueryStream`) intends.
</issue_description>
<issue_validation>
- **Checked:** `clearConversation` (claude-agent.ts:1544-1697), `retireQuery` (1480-1505), `withTimeout` (utils/common.ts:8-20), the consumer wiring (`ensureConsumer` 535-548), and the UI spinner lifecycle (buildConversationItems.ts:745-781, StatusNotificationView.tsx).
- **Found:** `withTimeout` only converts the *timeout* race outcome — `operationPromise = operation.then(...)` (common.ts:15-19), so a genuine rejection of the wrapped promise propagates. Both awaited `withTimeout` calls in the clear path can therefore reject non-timeout: `retireQuery`'s `await withTimeout(oldConsumer, 5000)` (1503) — and the consumer genuinely can reject (the codebase attaches a `.catch` logging "Consumer terminated unexpectedly" at 542) — and `await withTimeout(newQuery.initializationResult(), …)` (1584) on SDK subprocess crash / a concurrent abort of the live `newAbortController` (assigned to the session at 1582).
- **Found:** Only the `result.result === "timeout"` branch (1588-1604) recovers (terminateQuery + `queryClosed = true` + `clearing_failed`). There is no surrounding try/catch, and `session.query`/`input`/`queryOptions`/`abortController` are overwritten at 1579-1582 *before* the init await at 1584 — so a non-timeout rejection leaves the session pointing at a half-initialized query with `queryClosed` unset (so the guards at 1521/485 don't fire), and the `"clearing"` STATUS emitted at 1544-1547 is never resolved.
- **Found:** The UI confirms the stuck-spinner harm: the in-progress `clearing` row is flipped complete only by a `clearing` status with `isComplete` (buildConversationItems.ts:765-769) or a `clearing_failed` (771-780); nothing else (turn-end, error) clears it, so a non-timeout failure leaves "Clearing…" spinning permanently until a reload.
- **Impact:** A reachable failure mode (SDK init reject / consumer reject) leaves a permanently stuck spinner plus a half-swapped, non-`queryClosed` session that accepts further prompts into a dead query — directly defeating the PR's stated "failed clear gives the user feedback" hardening goal. Not defensive paranoia: these rejections are real (the code's own `.catch` acknowledges consumer rejection), and the author already handles the sibling timeout case identically, so extending it is completing intended behavior.
- **Priority:** Lowered must_fix → should_fix: the defect only triggers on an abnormal non-timeout failure during the clear swap (the success and timeout paths are both handled), so it degrades an already-failing operation rather than breaking the happy path; real and worth fixing, but not happy-path/data-loss severity.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Wrap the risky portion of clearConversation (from `retireQuery` through the notifications) in a try/catch (or try/finally) that, on ANY error — not just the timeout case — mirrors the timeout branch: call `terminateQuery` on the new query, set `session.queryClosed = true` (or otherwise restore a known-good state), and emit a `clearing_failed` STATUS notification before rethrowing. This guarantees the client never sees an unresolved "clearing" spinner and the session is never left half-swapped.
</potential_solution>
| if (commandMatch?.[1] === "/clear") { | ||
| // Handled by the adapter, never forwarded to the SDK (whose own /clear | ||
| // is unreliable in this embedding — see UPSTREAM.md "Hide /clear"). | ||
| return this.clearConversation(params); | ||
| } |
There was a problem hiding this comment.
/clear bypasses the turn-queue concurrency guard entirely, letting a concurrent /clear, prompt, or cancel race the in-flight session swap
Why we think it's a valid issue
- Checked: the
/clearshort-circuit inprompt()(claude-agent.ts:475-479) vs the normal turn-registration path (515-532),clearConversation's guard (1520-1549), theSessiontype (types.ts — fieldsturnQueue/activeTurn/queryClosed, noclearing),interrupt()(1387-1423), and the ACP transport's dispatch loop in@agentclientprotocol/sdk. - Found: The transport does not serialize per-session handlers —
jsonrpc.js:521-537receiveMessagefiresvoid this.processIncomingMessage(...)and thereceive()read-loop (487-505) immediately reads the next message without awaiting the handler, so twoprompt/cancelhandlers run concurrently, interleaving atawaitpoints. This is the same concurrency the codebase'sturnQueue/activeTurn/forceCancelTimermachinery exists to tame. - Found:
/clearreturnsthis.clearConversation(params)at 478 before theturnQueue.push(turn)at 529, so a clear registers nothing.clearConversation's sole guard (1524) checksactiveTurn !== null || turnQueue.length > 0and there is noclearingflag onSession— so a second concurrent/clearsees a clean state and also passes, andqueryClosedisn't set on the success path either (only at timeout 1592). TwoclearConversationbodies then race the samesession, both reassigningsession.query/input/abortController/sdkSessionId(1579-1582); the first clear's freshly-createdQueryis orphaned and never terminated (only the timeout branch at 1588-1604 callsterminateQuery), leaking that SDK subprocess/connection. - Found: A concurrent
cancel()→interrupt()(1422 operates on the livesession.query) or the abort of the just-installednewAbortController(1582) can reject the in-flight init, and a/clear+prompt overlap can push a user message to the old, already-end()-ed input fromretireQuery(1500), silently dropping it. - Impact: Confirmed race on shared mutable session state with a leaked SDK subprocess and possible silent message loss / corrupted swap — squarely the "race conditions that corrupt shared state" + "leaked connections" keep categories, and reachable via a plausible double-fire (double-click, client retry, or /clear immediately followed by another action) within the sub-second swap window; not a never-gonna-happen edge case given this codebase's concurrent dispatch model. The trivial fix (a synchronously-set
session.clearingflag, or registering /clear as a pseudo-turn) is well-scoped. - Priority: Lowered must_fix → should_fix: the common single-/clear flow is correctly guarded against in-flight normal turns, so the residual gap is the narrower concurrent clear/prompt/cancel timing window with restart-recoverable impact — real and worth fixing, but not happy-path/unrecoverable severity that must block merge.
Issue description
prompt() short-circuits /clear straight into clearConversation(params) (475-479) before any turnQueue/activeTurn registration happens — unlike every other prompt, which gets queued via session.turnQueue.push(turn) (see the normal path later in prompt()). clearConversation's only guard against concurrency is a one-time synchronous check of session.activeTurn !== null || session.turnQueue.length > 0 (1524), which protects against a normal turn being in flight but does nothing to prevent a second, overlapping clearConversation call: nothing on Session (checked in packages/agent/src/adapters/claude/types.ts) marks "a clear is currently in progress." The ACP transport (@agentclientprotocol/sdk's Connection.receive()/processIncomingMessage) dispatches every incoming JSON-RPC message via void this.processIncomingMessage(...) without awaiting the handler or serializing per-session — this codebase's own turnQueue/activeTurn/forceCancelTimer machinery exists specifically because concurrent prompt()+cancel() (and by extension a double /clear) is an expected, designed-for scenario, not a theoretical one. Concretely: a user double-sending /clear (double click, client retry), or sending /clear immediately followed by another prompt or a session/cancel, can run two overlapping clearConversation bodies (or a clearConversation racing interrupt()) against the same shared session object. Both mutate session.query, session.input, session.abortController, session.sdkSessionId, session.taskState, this.toolUseStreamCache, and this.emittedToolCalls without coordination — whichever call finishes last silently wins, and the "losing" call's freshly-created Query (and its underlying SDK subprocess/connection) is never terminated (only the timeout branch calls terminateQuery), leaking that resource. A concurrent cancel() mid-clear can also abort the very newAbortController that clearConversation just installed, feeding directly into the failure mode described in the previous finding.
Suggested fix
Add a session-level guard (e.g. a session.clearing: boolean, checked and set synchronously alongside the existing activeTurn/turnQueue check before the first await) so a second /clear — or a cancel() — arriving while one is already in flight is rejected or deferred instead of racing the swap. Alternatively, register /clear as a pseudo-Turn (push it onto turnQueue/activeTurn) so the existing turn-queue and forceCancelTimer machinery naturally serializes it the same way it already serializes normal prompts and cancels.
Prompt to fix with AI (copy-paste)
## Context
@packages/agent/src/adapters/claude/claude-agent.ts#L475-479
@packages/agent/src/adapters/claude/claude-agent.ts#L1520-1549
<issue_description>
`prompt()` short-circuits `/clear` straight into `clearConversation(params)` (475-479) before any `turnQueue`/`activeTurn` registration happens — unlike every other prompt, which gets queued via `session.turnQueue.push(turn)` (see the normal path later in `prompt()`). `clearConversation`'s only guard against concurrency is a one-time synchronous check of `session.activeTurn !== null || session.turnQueue.length > 0` (1524), which protects against a normal turn being in flight but does nothing to prevent a second, overlapping `clearConversation` call: nothing on `Session` (checked in `packages/agent/src/adapters/claude/types.ts`) marks "a clear is currently in progress." The ACP transport (`@agentclientprotocol/sdk`'s `Connection.receive()`/`processIncomingMessage`) dispatches every incoming JSON-RPC message via `void this.processIncomingMessage(...)` without awaiting the handler or serializing per-session — this codebase's own `turnQueue`/`activeTurn`/`forceCancelTimer` machinery exists specifically because concurrent `prompt()`+`cancel()` (and by extension a double `/clear`) is an expected, designed-for scenario, not a theoretical one. Concretely: a user double-sending `/clear` (double click, client retry), or sending `/clear` immediately followed by another prompt or a `session/cancel`, can run two overlapping `clearConversation` bodies (or a `clearConversation` racing `interrupt()`) against the *same* shared `session` object. Both mutate `session.query`, `session.input`, `session.abortController`, `session.sdkSessionId`, `session.taskState`, `this.toolUseStreamCache`, and `this.emittedToolCalls` without coordination — whichever call finishes last silently wins, and the "losing" call's freshly-created `Query` (and its underlying SDK subprocess/connection) is never terminated (only the timeout branch calls `terminateQuery`), leaking that resource. A concurrent `cancel()` mid-clear can also abort the very `newAbortController` that `clearConversation` just installed, feeding directly into the failure mode described in the previous finding.
</issue_description>
<issue_validation>
- **Checked:** the `/clear` short-circuit in `prompt()` (claude-agent.ts:475-479) vs the normal turn-registration path (515-532), `clearConversation`'s guard (1520-1549), the `Session` type (types.ts — fields `turnQueue`/`activeTurn`/`queryClosed`, no `clearing`), `interrupt()` (1387-1423), and the ACP transport's dispatch loop in `@agentclientprotocol/sdk`.
- **Found:** The transport does not serialize per-session handlers — `jsonrpc.js:521-537` `receiveMessage` fires `void this.processIncomingMessage(...)` and the `receive()` read-loop (487-505) immediately reads the next message without awaiting the handler, so two `prompt`/`cancel` handlers run concurrently, interleaving at `await` points. This is the same concurrency the codebase's `turnQueue`/`activeTurn`/`forceCancelTimer` machinery exists to tame.
- **Found:** `/clear` returns `this.clearConversation(params)` at 478 before the `turnQueue.push(turn)` at 529, so a clear registers nothing. `clearConversation`'s sole guard (1524) checks `activeTurn !== null || turnQueue.length > 0` and there is no `clearing` flag on `Session` — so a second concurrent `/clear` sees a clean state and also passes, and `queryClosed` isn't set on the success path either (only at timeout 1592). Two `clearConversation` bodies then race the same `session`, both reassigning `session.query`/`input`/`abortController`/`sdkSessionId` (1579-1582); the first clear's freshly-created `Query` is orphaned and never terminated (only the timeout branch at 1588-1604 calls `terminateQuery`), leaking that SDK subprocess/connection.
- **Found:** A concurrent `cancel()`→`interrupt()` (1422 operates on the live `session.query`) or the abort of the just-installed `newAbortController` (1582) can reject the in-flight init, and a `/clear`+prompt overlap can push a user message to the old, already-`end()`-ed input from `retireQuery` (1500), silently dropping it.
- **Impact:** Confirmed race on shared mutable session state with a leaked SDK subprocess and possible silent message loss / corrupted swap — squarely the "race conditions that corrupt shared state" + "leaked connections" keep categories, and reachable via a plausible double-fire (double-click, client retry, or /clear immediately followed by another action) within the sub-second swap window; not a never-gonna-happen edge case given this codebase's concurrent dispatch model. The trivial fix (a synchronously-set `session.clearing` flag, or registering /clear as a pseudo-turn) is well-scoped.
- **Priority:** Lowered must_fix → should_fix: the common single-/clear flow is correctly guarded against in-flight normal turns, so the residual gap is the narrower concurrent clear/prompt/cancel timing window with restart-recoverable impact — real and worth fixing, but not happy-path/unrecoverable severity that must block merge.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Add a session-level guard (e.g. a `session.clearing: boolean`, checked and set synchronously alongside the existing `activeTurn`/`turnQueue` check before the first `await`) so a second `/clear` — or a `cancel()` — arriving while one is already in flight is rejected or deferred instead of racing the swap. Alternatively, register `/clear` as a pseudo-`Turn` (push it onto `turnQueue`/`activeTurn`) so the existing turn-queue and `forceCancelTimer` machinery naturally serializes it the same way it already serializes normal prompts and cancels.
</potential_solution>
Problem
Typing
/clearin PostHog Code reports "Unsupported slash command". Upstream hid the SDK's own/clearbecause it is unreliable in this embedding, but users still want a way to drop the conversation and free up context without abandoning the task, including on cloud runs, where the append-only session log is what rehydrates the conversation on resume.Here's an example session that shows it works.
Refs #670
Changes
packages/agent):prompt()intercepts/clear(never forwarded to the SDK). It retires the current SDK query (reusing therefreshSessionteardown, now extracted asretireQuery) and swaps in a brand-new SDK session, fresh id, no resume, under the same ACP session. The ACP session id stays stable; the newSession.sdkSessionIdtracks the underlying SDK session, andhasSessionaccepts either id so cancel/reconnect work from both sides./clearis advertised in the command list again._posthog/conversation_clearedmarker plus an updatedsdk_sessionmapping in the append-only log.rebuildConversation(jsonl hydration +ResumeSaga) treats the marker as a boundary, andResumeSaga.findSessionIdpicks up the post-clear session id, so cloud sandbox restarts and desktop cold reconnects resume the fresh SDK session with only post-clear turns.usage_update./clearprompt is only broadcast (and thus logged) once the new session is confirmed live, so a timeout leaves no orphaned entry in the log. A "Clearing…" status indicator mirrors the existing compaction spinner, so a slow or failed clear gives the user feedback instead of going silent, with aclearing_failedrow rendering the outcome. On success, the local session file left under the original session id gets deleted so a cold reconnect can't mistake it for current and resume the pre-clear conversation./clearalso now refuses while a turn is queued, not just while one is active.Codex sessions are unchanged,
/clearis Claude-adapter only for now.How did you test this?
/clearbehavior (fresh session swap, marker/mapping emission, busy/queued-turn guards, refresh-after-clear, timeout handling, stale jsonl cleanup, resumeSession id matching after a clear), command advertisement, clear-boundary handling in jsonl hydration andResumeSaga, and the UI divider and status-row rendering (includingclearing/clearing_failed).@posthog/agentsuite,@posthog/uisessions suite, andpnpm typecheckfor agent/core/ui plus Biome on the touched packages.Automatic notifications
Why:
/clearis a frequently reached-for command that PostHog Code currently rejects; implementing it cloud-first required making the session-log rehydration clear-aware.Created with PostHog Code