feat(cli): migrate conversations to thin IPC wrapper#30242
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| async function handleClearCli(_args: RouteHandlerArgs) { | ||
| // Tear down in-memory conversation state before DB clear. | ||
| const cleared = clearAllActive(); | ||
| log.info({ cleared }, "CLI conversations clear: active conversations torn down"); | ||
| return { cleared }; |
There was a problem hiding this comment.
🔴 Qdrant vector collection no longer deleted by conversations clear command
The old clear command explicitly initialized a Qdrant client and called qdrant.deleteCollection() to delete the entire vector collection (old conversations.ts lines 303–324). The new handleClearCli handler at assistant/src/runtime/routes/conversation-cli-routes.ts:109-113 only calls clearAllConversations() (which clears in-memory state + SQLite DB) but never touches Qdrant. This leaves orphaned vector data behind.
The CLI help text at assistant/src/cli/commands/conversations.ts:241 still describes the command as "Clear all conversations, messages, and vector data" and the confirmation prompt at line 258 still says "This will permanently delete all conversations, messages, and vector data" — but vector data is no longer being deleted. This is a silent behavioral regression that violates the backwards-compatibility rule in AGENTS.md.
Prompt for agents
The handleClearCli handler in assistant/src/runtime/routes/conversation-cli-routes.ts only calls clearAllConversations() (which clears in-memory state and SQLite) but does not delete the Qdrant vector collection. The old CLI clear command in assistant/src/cli/commands/conversations.ts explicitly initialized a Qdrant client (via initQdrantClient with config from getConfig()) and called qdrant.deleteCollection() to remove the vector store.
To fix this, the handleClearCli handler needs to also delete the Qdrant collection after clearing the DB. This likely involves importing the Qdrant client initialization helpers (resolveQdrantUrl, initQdrantClient from memory/qdrant-client.ts) and the embedding backend selector (selectEmbeddingBackend from memory/embedding-backend.ts), reading the relevant config, and calling deleteCollection(). Since the handler runs inside the daemon process, it has full access to config and Qdrant.
Alternatively, if Qdrant cleanup is intentionally being deferred, the CLI help text at assistant/src/cli/commands/conversations.ts lines 241 and 258 must be updated to remove the 'vector data' promise.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| import { z } from "zod"; | ||
|
|
||
| import { clearAllConversations as clearAllActive } from "../../daemon/handlers/conversations.js"; |
There was a problem hiding this comment.
🚩 Misleading import alias clearAllActive hides DB clearing
At assistant/src/runtime/routes/conversation-cli-routes.ts:11, clearAllConversations is aliased as clearAllActive. This alias suggests only in-memory (active) conversations are cleared, but clearAllConversations at assistant/src/daemon/handlers/conversations.ts:52-59 also calls clearAll() from conversation-crud which deletes all DB rows. The alias is misleading but functionally correct — DB clearing does happen. However, the return value (cleared) is the count of active in-memory conversations only (from clearAllActiveConversations()), not the total DB conversation count. The old code reported total DB counts (result.conversations + result.messages). The new CLI at assistant/src/cli/commands/conversations.ts:282 prints Cleared ${result.result!.cleared} conversations which may undercount if there are DB-only (evicted) conversations.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e711f6b06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| operationId: "conversations_clear_cli", | ||
| endpoint: "conversations/cli/clear", | ||
| method: "POST", | ||
| summary: "Clear all conversations (CLI)", | ||
| description: | ||
| "Tear down all active conversations and clear the database. " + | ||
| "The confirmation prompt is handled client-side by the CLI.", | ||
| tags: ["conversations"], |
There was a problem hiding this comment.
conversations/cli/clear is registered in the shared runtime ROUTES, so it is exposed over HTTP as well as IPC, but this route has no policyKey and there is no conversations/cli/clear entry in route-policy.ts; in http-router.ts, missing policy entries are allowed. That means any authenticated caller that can hit HTTP can bypass the existing conversations/clear-all safeguards (including its stricter scope gate) and still wipe all conversations via this endpoint.
Useful? React with 👍 / 👎.
| async function handleClearCli(_args: RouteHandlerArgs) { | ||
| // Tear down in-memory conversation state before DB clear. | ||
| const cleared = clearAllActive(); | ||
| log.info({ cleared }, "CLI conversations clear: active conversations torn down"); | ||
| return { cleared }; |
There was a problem hiding this comment.
The new clear handler only calls clearAllConversations() and returns, so assistant conversations clear no longer removes Qdrant data even though the command still advertises clearing vector data. The previous CLI path explicitly deleted the Qdrant collection; without equivalent cleanup here, stale vectors persist (and can still be surfaced for payload types that are not DB-validated) and storage keeps growing after repeated clears.
Useful? React with 👍 / 👎.
| }>("wipe_conversation", { | ||
| body: { conversationId }, | ||
| }); |
There was a problem hiding this comment.
This command now forwards the user-provided conversationId directly to wipe_conversation, but that route performs an exact getConversation(conversationId) lookup. Before this change, the CLI resolved prefixes locally first; now inputs like abc123 (documented as valid prefix matches) fail with not found unless the full ID is provided.
Useful? React with 👍 / 👎.
…ation/pr-3 # Conflicts: # assistant/src/cli/COMMAND_INVENTORY.md
) The CLI IPC refactor PRs (#30238–#30247) added new shared routes without matching policy entries in route-policy.ts. The route-policy coverage guard test fails on main with 29 missing endpoints: - audit, auth/info - conversations/cli/{list,create,export,clear} - inference/send - internal/mcp/{list,add,remove} - oauth/apps.upsert, oauth/apps/lookup - oauth/providers.{register,update,delete} - platform/{status,connect,disconnect,callback-routes/register,callback-routes} - sequences/{list,get,pause,resume,cancel-enrollment,stats,guardrails} - user-routes/{list,inspect} http-router.ts treats unregistered endpoints as unprotected (enforcePolicy no-ops on missing keys), so any authenticated principal could hit these over HTTP without the intended scope/principal-type gates. The IPC dispatch path bypasses enforcePolicy entirely, so the existing CLI flows are unaffected by these additions. Scope/principal assignments mirror sibling routes: - oauth/apps mutations use ACTOR_ENDPOINTS settings.write (matches oauth/apps.create/.delete already present) - oauth/providers mutations use ACTOR_ENDPOINTS settings.write - oauth/apps/lookup uses ACTOR_ENDPOINTS settings.read - auth/info uses ACTOR_ENDPOINTS settings.read - internal/mcp/{list,add,remove} join INTERNAL_ENDPOINTS (gateway-only, internal.write) alongside the existing internal/mcp/reload entry - CLI-driven routes (audit, conversations/cli/*, inference/send, platform/*, sequences/*, user-routes/*) are local-only with settings.read or settings.write, mirroring tasks/*, cache/*, defer/*, domain/*, email/* — destructive 'conversations/cli/clear' elevates to settings.write to match conversations/clear-all - inference/send uses chat.write (LLM dispatch on behalf of caller), aligned with tts/synthesize-cli + stt/transcribe-file Closes route-policy coverage findings on PRs #30241 (sequences, user-routes), #30242 (conversations/cli/clear), #30244 (inference/send), #30245 (internal/mcp), #30246 (oauth/providers, oauth/apps), #30247 (platform/*). Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com>
1 participant
Summary
Part of plan: cli-ipc-migration.md (PR 3 of 11)