feat: add multi-provider chat imports - #7160
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| continue; | ||
| } | ||
| importedHermesSessionIds.add(hermesSessionId); | ||
| imported += 1; |
There was a problem hiding this comment.
Partial import blocks retry forever
High Severity
Import creates a thread with a deterministic id, then imports history and upserts the provider binding. If anything after thread.create fails, the session is never marked imported, but the empty thread remains. A later import hits requireThreadAbsent and permanently fails that session.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 44275cd. Configure here.
| ...(input.rawOutput !== undefined ? { rawOutput: input.rawOutput } : {}), | ||
| ...(command ? { command } : {}), | ||
| ...(stdout ? { stdout } : {}), | ||
| ...(files.length > 0 ? { files } : {}), |
There was a problem hiding this comment.
Imported file paths dropped on project
High Severity
Codex/OpenCode imports store activity data.files as a bare string[]. Client-facing projection rebuilds files via collectChangedFiles, which only keeps object entries with path-like keys and does not walk rawInput/rawOutput. Projected work-log rows therefore lose changed-file paths for local imports.
Reviewed by Cursor Bugbot for commit 44275cd. Configure here.
| createdAt, | ||
| }; | ||
| yield* engine.dispatch(projectCommand); | ||
| projectsByRoot.set(workspaceRoot, projectId); |
There was a problem hiding this comment.
Hermes project root key mismatch
Medium Severity
Hermes import keys projectsByRoot and session cwd/git_repo_root without path.resolve or home expansion, unlike the local importer. The same workspace can miss the map lookup, attempt a second project.create, and fail the active-workspace-root invariant—or land chats under the ungrouped import project.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 44275cd. Configure here.
| return previous === undefined || previous.title !== next.title || previous.detail !== next.detail; | ||
| } | ||
|
|
||
| const stdoutDecoder = new TextDecoder(); |
There was a problem hiding this comment.
🟠 High acp/AcpSessionRuntime.ts:978
Concurrent filtered ACP stdout streams share stdoutDecoder, so when one stream ends a chunk with a partial multibyte UTF-8 character, the next stream's bytes are decoded with that retained state and its JSON-RPC output is corrupted, causing protocol parsing and session failures. Create the TextDecoder inside each discardNonJsonStdoutLines stream instance and flush it at end-of-stream so decoder state cannot cross sessions and trailing bytes are not lost.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/acp/AcpSessionRuntime.ts around line 978:
Concurrent filtered ACP stdout streams share `stdoutDecoder`, so when one stream ends a chunk with a partial multibyte UTF-8 character, the next stream's bytes are decoded with that retained state and its JSON-RPC output is corrupted, causing protocol parsing and session failures. Create the `TextDecoder` inside each `discardNonJsonStdoutLines` stream instance and flush it at end-of-stream so decoder state cannot cross sessions and trailing bytes are not lost.
| const value = asRecord(parseJson(part.data)); | ||
| if (!partId || value?.type !== "tool") continue; | ||
| const state = asRecord(value.state); | ||
| const status = stringValue(state?.status); |
There was a problem hiding this comment.
🟡 Medium provider/localChatImport.ts:438
pending and running OpenCode tool parts are imported with status: "completed", so interrupted or active calls appear as successfully completed in the thread history. Because only error and failed set failed: true, skip nonterminal states (or preserve their actual status) before calling activityFromTool.
const status = stringValue(state?.status);
+ if (status !== "completed" && status !== "error" && status !== "failed") continue;🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/localChatImport.ts around line 438:
`pending` and `running` OpenCode tool parts are imported with `status: "completed"`, so interrupted or active calls appear as successfully completed in the thread history. Because only `error` and `failed` set `failed: true`, skip nonterminal states (or preserve their actual status) before calling `activityFromTool`.
| ? ({ outcome: "cancelled" } as const) | ||
| : { | ||
| outcome: "selected" as const, | ||
| optionId: acpPermissionOutcome(resolved), |
There was a problem hiding this comment.
🟠 High Layers/HermesAdapter.ts:633
Manual permission responses send hard-coded IDs from acpPermissionOutcome(resolved), so Hermes requests with agent-defined optionId values return an invalid selection and fail instead of applying the user's approval or rejection. Select the matching params.options entry by decision/kind and return its optionId, as the Grok adapter does.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/HermesAdapter.ts around line 633:
Manual permission responses send hard-coded IDs from `acpPermissionOutcome(resolved)`, so Hermes requests with agent-defined `optionId` values return an invalid selection and fail instead of applying the user's approval or rejection. Select the matching `params.options` entry by decision/kind and return its `optionId`, as the Grok adapter does.
| command, | ||
| threadId: command.threadId, | ||
| }); | ||
| const updatedAt = [...command.messages, ...command.activities].reduce( |
There was a problem hiding this comment.
🟡 Medium orchestration/decider.ts:1317
An empty thread.history.import emits thread.history-imported with updatedAt and occurredAt set to 1970-01-01T00:00:00.000Z, rewinding the existing thread and making it sort as decades old. Reject empty imports before the reduction so the sentinel is never persisted.
Also found in 1 other location(s)
packages/contracts/src/orchestration.ts:1008
ThreadHistoryImportCommandaccepts a history with bothmessagesandactivitiesempty. On that valid input, the decider's max-date reduction falls back to1970-01-01T00:00:00.000Z, emitsthread.history-imported, and the projector replaces the existing thread'supdatedAtwith that epoch value. An empty/fully-filtered imported session can therefore make an existing thread appear decades old and disrupt updated-time ordering; require at least one imported item or make an empty import a no-op.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/decider.ts around line 1317:
An empty `thread.history.import` emits `thread.history-imported` with `updatedAt` and `occurredAt` set to `1970-01-01T00:00:00.000Z`, rewinding the existing thread and making it sort as decades old. Reject empty imports before the reduction so the sentinel is never persisted.
Also found in 1 other location(s):
- packages/contracts/src/orchestration.ts:1008 -- `ThreadHistoryImportCommand` accepts a history with both `messages` and `activities` empty. On that valid input, the decider's max-date reduction falls back to `1970-01-01T00:00:00.000Z`, emits `thread.history-imported`, and the projector replaces the existing thread's `updatedAt` with that epoch value. An empty/fully-filtered imported session can therefore make an existing thread appear decades old and disrupt updated-time ordering; require at least one imported item or make an empty import a no-op.
| // Count this prompt immediately so a superseded in-flight prompt | ||
| // resolving from here on does not settle the turn; the matching | ||
| // decrement is the `ensuring` below. | ||
| ctx.promptsInFlight += 1; |
There was a problem hiding this comment.
🟠 High Layers/HermesAdapter.ts:829
A steering sendTurn arriving during applyRequestedSessionConfiguration gets a new turnId and emits a second turn.started, so the prompts are reported as separate turns instead of being merged. ctx.promptsInFlight is incremented before that yielding configuration step, but ctx.activeTurnId is assigned afterward; bind the new turn ID immediately after incrementing, before configuration begins.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/HermesAdapter.ts around line 829:
A steering `sendTurn` arriving during `applyRequestedSessionConfiguration` gets a new `turnId` and emits a second `turn.started`, so the prompts are reported as separate turns instead of being merged. `ctx.promptsInFlight` is incremented before that yielding configuration step, but `ctx.activeTurnId` is assigned afterward; bind the new turn ID immediately after incrementing, before configuration begins.
| const failed = | ||
| input.failed === true || | ||
| outputRecord?.error !== undefined || | ||
| outputRecord?.status === "failed" || | ||
| outputRecord?.status === "error"; |
There was a problem hiding this comment.
🟡 Medium provider/localChatImport.ts:205
Tool results with error: null are imported as failed, while command results with a nonzero exit_code are imported as successfully completed, so chat history misrepresents both successful and failed executions. This happens because the check treats any defined error value as failure and ignores exit_code; only non-null errors and nonzero exit codes should set the failure status.
- const failed =
- input.failed === true ||
- outputRecord?.error !== undefined ||
- outputRecord?.status === "failed" ||
- outputRecord?.status === "error";
+ const failed =
+ input.failed === true ||
+ outputRecord?.error != null ||
+ (outputRecord?.exit_code != null && Number(outputRecord.exit_code) !== 0) ||
+ outputRecord?.status === "failed" ||
+ outputRecord?.status === "error";🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/localChatImport.ts around lines 205-209:
Tool results with `error: null` are imported as failed, while command results with a nonzero `exit_code` are imported as successfully completed, so chat history misrepresents both successful and failed executions. This happens because the check treats any defined `error` value as failure and ignores `exit_code`; only non-null errors and nonzero exit codes should set the failure status.
| // is snoozed (the count is the whole footprint when | ||
| // collapsed); rows only when expanded. Vanishes entirely at | ||
| // count 0. | ||
| if (snoozedThreads.length > 0) { |
There was a problem hiding this comment.
🟡 Medium components/Sidebar.tsx:3905
The Snoozed and Settled headers no longer delimit their rows: hierarchyItems renders pinned, active, snoozed, and settled threads after both shelf headers, so active rows appear under the shelf labels and expanding a shelf does not reveal its corresponding content. Preserve the section boundaries when rendering the provider/project hierarchy so each shelf header remains immediately associated with its rows.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/Sidebar.tsx around line 3905:
The `Snoozed` and `Settled` headers no longer delimit their rows: `hierarchyItems` renders pinned, active, snoozed, and settled threads after both shelf headers, so active rows appear under the shelf labels and expanding a shelf does not reveal its corresponding content. Preserve the section boundaries when rendering the provider/project hierarchy so each shelf header remains immediately associated with its rows.
| label: "environment-data:server:import-hermes-sessions", | ||
| tag: WS_METHODS.serverImportHermesSessions, | ||
| concurrency: { | ||
| mode: "singleFlight", |
There was a problem hiding this comment.
🟡 Medium state/server.ts:738
Concurrent imports for different provider instances are deduplicated, so the second importHermesSessions or importLocalChats request receives the first result and its requested instance is never imported. Both singleFlight keys omit input.instanceId; include it using an unambiguous tuple serialization.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/server.ts around line 738:
Concurrent imports for different provider instances are deduplicated, so the second `importHermesSessions` or `importLocalChats` request receives the first result and its requested instance is never imported. Both `singleFlight` keys omit `input.instanceId`; include it using an unambiguous tuple serialization.
| ...(eventLoggers.native ? { nativeEventLogger: eventLoggers.native } : {}), | ||
| instanceId, | ||
| }); | ||
| const textGeneration = makeHermesTextGeneration(); |
There was a problem hiding this comment.
🟡 Medium Drivers/HermesDriver.ts:110
When Hermes is the only enabled provider, automatic thread titles, branch names, and commit/PR generation fail because fallbackTextGenerationProvider selects this instance and makeHermesTextGeneration() rejects every operation as unsupported. Hermes must implement text generation or be excluded from text-generation fallback selection.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Drivers/HermesDriver.ts around line 110:
When Hermes is the only enabled provider, automatic thread titles, branch names, and commit/PR generation fail because `fallbackTextGenerationProvider` selects this instance and `makeHermesTextGeneration()` rejects every operation as unsupported. Hermes must implement text generation or be excluded from text-generation fallback selection.
There was a problem hiding this comment.
Effect service conventions review of the new Hermes provider / chat-import modules. Three findings, all in the error-modeling rules: two prohibited pure error-construction helpers with free-text reason payloads, and one error that copies raw child-process stderr into a caller-visible message. The service acquisition, layer, namespace-import, and file-layout conventions look fine in the touched code (dependencies are acquired with yield*, no ManagedRuntime/runPromise in service code, no compatibility shims).
Posted via Macroscope — Effect Service Conventions
| function importError(platform: LocalChatImportPlatform, reason: string, cause?: unknown) { | ||
| return new LocalChatImportError({ | ||
| platform, | ||
| reason, | ||
| ...(cause === undefined ? {} : { cause }), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Same pattern here: importError is a pure new LocalChatImportError({ ... }) wrapper used only with mapError, and reason is a prose sentence that is the error's only data. Constructing the error where the failure happens, with structural attributes (stage/operation and the session id or database/transcript path — e.g. the Could not read '${databasePath}'. case at loadOpenCodeSessions, which also wraps open + three queries + close in one generic error), keeps the failures diagnosable and lets message be derived from those fields.
Posted via Macroscope — Effect Service Conventions
| function importError(reason: string, cause?: unknown) { | ||
| return new HermesImportSessionsError({ | ||
| reason, | ||
| ...(cause === undefined ? {} : { cause }), | ||
| }); |
There was a problem hiding this comment.
importError does nothing but forward its arguments into HermesImportSessionsError, which the conventions call out explicitly (no helpers of the form (...args) => new SomeError({ ...args }), including aliases used with mapError). Every call site also passes a prose sentence as the only payload, so the error carries no stable structure even where the context is known (exporter stage, Hermes session id, instance id, path).
Consider constructing the error at each failure boundary and adding stable fields to the class in packages/contracts/src/hermes.ts (e.g. stage plus sessionId/instanceId), deriving message from those attributes instead of a free-text reason.
Posted via Macroscope — Effect Service Conventions
| if (result.code !== 0) { | ||
| return yield* importError( | ||
| result.stderr.trim() || `Hermes session exporter exited with code ${result.code}.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Raw child-process stderr is copied into reason, which becomes the caller-visible message (it is surfaced verbatim in the settings toast). Error attributes should stay bounded and safe: keep normalized diagnostics (exit code, stderrLength) as structural fields and leave the exact output to cause/logs — the same approach providerSnapshot.ts and VcsProcess.ts already use.
| if (result.code !== 0) { | |
| return yield* importError( | |
| result.stderr.trim() || `Hermes session exporter exited with code ${result.code}.`, | |
| ); | |
| } | |
| if (result.code !== 0) { | |
| return yield* importError(`Hermes session exporter exited with code ${result.code}.`); | |
| } |
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
UI Consistency: findings
Six issues in the changed web UI code, mostly in the sidebar restructure. The largest are the detached shelf headers, the undefined sidebar-accent token on the new row actions, and the settled-paging/DnD ordering mismatches introduced by the platform hierarchy.
Posted via Macroscope — UI Consistency
| { | ||
| value: ProviderDriverKind.make("hermes"), | ||
| label: "Hermes", | ||
| icon: PiAgentIcon, |
There was a problem hiding this comment.
PiAgentIcon is the Pi Agent brand glyph and is already used to identify the piAgent driver in AddProviderInstanceDialog.tsx (line 99). Reusing it for Hermes means one glyph now identifies two different providers across the provider rows, the instance icon, and the new sidebar platform headers.
Consider adding a Hermes glyph to components/Icons.tsx (or falling back to a neutral icon) so provider identity stays unambiguous.
Posted via Macroscope — UI Consistency
| } | ||
| disabled={archiveDisabled} | ||
| onClick={handleArchiveClick} | ||
| className="inline-flex size-6 shrink-0 cursor-pointer items-center justify-center rounded-md text-muted-foreground opacity-0 outline-none transition-opacity hover:bg-sidebar-accent hover:text-foreground focus-visible:opacity-100 focus-visible:ring-2 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-30 group-hover/sidebar-row:opacity-100" |
There was a problem hiding this comment.
In the card row the sibling hover controls (snooze/settle, line 1514) stay absolute and only become static on hover/focus so they consume no layout space at rest. These two new buttons are always in flow with opacity-0, so ~3rem of the metadata line is permanently reserved and the status/time text truncates earlier even when the row is not hovered (the compact row has the same problem, partly masked by -ml-2).
Consider matching the existing reveal pattern — wrap the pair in the same pointer-events-none absolute inset-y-0 right-0 … group-hover/sidebar-row:static container — and use hover:bg-sidebar-row-hover instead of the undefined bg-sidebar-accent.
Posted via Macroscope — UI Consistency
| } | ||
| for (const thread of renderedSettledThreads) { | ||
| items.push(renderThreadRow(thread, "settled")); | ||
| const hierarchyItems: ReactNode[] = []; |
There was a problem hiding this comment.
The snoozed and settled shelf headers are pushed before the hierarchy, but every row — pinned, active, snoozed and settled alike — is now emitted inside hierarchyItems. The two collapsible headers therefore render stacked at the top of the list above content they do not label, aria-expanded no longer describes adjacent rows, and toggling a shelf adds/removes rows from arbitrary positions further down inside platform/project groups. The previous pinned divider that separated the pinned block from the inbox is gone as well.
Smallest fix: build the platform/project hierarchy per section and push each section's header immediately before its own grouped rows, so header → rows adjacency (and the toggle semantics) is preserved.
Posted via Macroscope — UI Consistency
| } | ||
| disabled={archiveDisabled} | ||
| onClick={handleArchiveClick} | ||
| className="-ml-2 inline-flex size-7 shrink-0 cursor-pointer items-center justify-center rounded-md text-muted-foreground opacity-0 outline-none transition-opacity hover:bg-sidebar-accent hover:text-foreground focus-visible:opacity-100 focus-visible:ring-2 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-30 group-hover/sidebar-row:opacity-100" |
There was a problem hiding this comment.
bg-sidebar-accent is not a token in this app's Tailwind theme (apps/web/src/index.css defines --color-sidebar-row-hover, --color-sidebar-row-active, --color-sidebar-control-surface, but no --color-sidebar-accent, and nothing else in the repo uses it), so no rule is emitted and these buttons get no hover background — unlike every other sidebar hover surface. Same class appears on the actions button below and on both card-row buttons (1555, 1564).
Consider using the sidebar hover token, and — since this durable micro-icon geometry is now repeated four times — hoisting it into one constant the way LegacySidebar.tsx does with SIDEBAR_ICON_ACTION_BUTTON_CLASS.
| className="-ml-2 inline-flex size-7 shrink-0 cursor-pointer items-center justify-center rounded-md text-muted-foreground opacity-0 outline-none transition-opacity hover:bg-sidebar-accent hover:text-foreground focus-visible:opacity-100 focus-visible:ring-2 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-30 group-hover/sidebar-row:opacity-100" | |
| className="-ml-2 inline-flex size-7 shrink-0 cursor-pointer items-center justify-center rounded-md text-muted-foreground opacity-0 outline-none transition-opacity hover:bg-sidebar-row-hover hover:text-foreground focus-visible:opacity-100 focus-visible:ring-2 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-30 group-hover/sidebar-row:opacity-100" |
Posted via Macroscope — UI Consistency
| bucket.push(thread); | ||
| buckets.set(key, bucket); | ||
| } | ||
| const visible = [...buckets.values()].flatMap((bucket) => bucket.slice(0, settledVisibleCount)); |
There was a problem hiding this comment.
The visible settled set is now sliced per platform bucket, but the route-thread rescue below still slices settledThreads globally (settledThreads.slice(settledVisibleCount)). A thread past the global cutoff can already be inside its own bucket's first settledVisibleCount items, so it gets pushed a second time — the same thread then renders twice in its project group with a duplicate React key. hiddenSettledCount (and therefore the Show more affordance) is also derived from this mixed count.
Smallest fix: build a Set of visible thread keys from the bucketed result and only push the route thread when it is absent, then derive hiddenSettledCount from that set. (The button label also dropped its count — Show {n} more conveyed how much tail remains.)
Posted via Macroscope — UI Consistency
| .filter((threadKey) => reorderablePinnedKeys.has(threadKey))} | ||
| strategy={verticalListSortingStrategy} | ||
| > | ||
| {hierarchyItems} |
There was a problem hiding this comment.
SortableContext still receives items in orderedPinnedThreads order, but the rendered pinned rows are now scattered across platform and project groups and are no longer contiguous or in that order. handlePinnedDragEnd computes the new index with arrayMove over that array while dnd-kit measures the rendered rects, so a drop onto a pinned row in another platform group reorders something the user did not see move (and the sorting animation crosses non-sortable header rows).
Smallest fix: keep pinned rows in a single contiguous block, or scope one SortableContext per group and ignore cross-group drops.
Posted via Macroscope — UI Consistency
ApprovabilityVerdict: Needs human review 12 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
| </button> | ||
| </li>, | ||
| ); | ||
| if (!platformExpanded) continue; |
There was a problem hiding this comment.
🟡 Medium components/Sidebar.tsx:4012
Collapsing a platform or project hides its rows but does not remove them from orderedThreadKeys/threadByKey, so thread-jump and previous/next shortcuts still navigate to hidden threads, and selections made before collapsing remain eligible for bulk actions. Build traversal/actionable collections from only expanded platform/project descendants, or clear/filter selection when either hierarchy level collapses.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/Sidebar.tsx around line 4012:
Collapsing a platform or project hides its rows but does not remove them from `orderedThreadKeys`/`threadByKey`, so thread-jump and previous/next shortcuts still navigate to hidden threads, and selections made before collapsing remain eligible for bulk actions. Build traversal/actionable collections from only expanded platform/project descendants, or clear/filter selection when either hierarchy level collapses.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b04ceae. Configure here.
| </button> | ||
| </li>, | ||
| ); | ||
| if (!projectExpanded) continue; |
There was a problem hiding this comment.
Collapsed threads stay selectable
Medium Severity
Platform and project collapse only skips DOM rendering with continue, while those threads remain in orderedThreadKeys. Settled and snoozed shelf collapse already removes collapsed rows from that list so jump shortcuts and shift-range multi-select ignore them. Collapsed platform or project chats stay in shortcut indices and range selection, so actions can target threads that are not visible.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b04ceae. Configure here.
| </button> | ||
| </li>, | ||
| ); | ||
| if (!projectExpanded) continue; |
There was a problem hiding this comment.
Open thread hidden on collapse
Medium Severity
Collapsing a platform or project hides every child row, including the currently open thread. Settled and snoozed shelves already keep the route thread visible when collapsed so the highlight and actions stay reachable. After this change, collapsing the parent of the active chat removes that row with no equivalent exception.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b04ceae. Configure here.
There was a problem hiding this comment.
UI Consistency
Two new findings in the collapsible platform/project hierarchy added by fix(web): collapse platform and project chats. Both are posted inline.
Previously reported findings that are still present in apps/web/src/components/Sidebar.tsx (not re-posted, see the earlier review threads):
hover:bg-sidebar-accenton the new archive/actions row buttons is not a token in this app's theme, so those controls get no hover surface (lines 1344, 1353, 1555, 1564).- The new archive/actions buttons sit in flow with
opacity-0instead of following the existingabsolute→static-on-hover reveal used by the sibling snooze/settle control, permanently reserving metadata-line width. - The Snoozed/Settled shelf headers are emitted before the hierarchy while their rows now render inside platform/project groups, so the headers no longer label adjacent content and
aria-expandeddescribes nothing next to them. SortableContextstill receivesitemsinorderedPinnedThreadsorder while pinned rows are scattered across groups, so pinned drag-reorder can move rows the user did not see.PiAgentIconis reused for the Hermes driver even though it already identifies thepiAgentdriver.
The settled-tail duplicate-row issue from the earlier review is fixed by the global settledThreads.slice(...) restoration.
Posted via Macroscope — UI Consistency
| > | ||
| <PlusIcon aria-hidden className="size-4 shrink-0" /> | ||
| Show {Math.min(hiddenSettledCount, SETTLED_TAIL_PAGE_COUNT)} more | ||
| Show more |
There was a problem hiding this comment.
Now that visibleSettledThreads slices settledThreads globally again, hiddenSettledCount is accurate, so the page count no longer needs to be dropped from this label — every other affordance in this list (shelf headers, platform and project headers) shows a count.
| Show more | |
| Show {Math.min(hiddenSettledCount, SETTLED_TAIL_PAGE_COUNT)} more |
Posted via Macroscope — UI Consistency
| </button> | ||
| </li>, | ||
| ); | ||
| if (!platformExpanded) continue; |
There was a problem hiding this comment.
Collapsing a platform (and likewise a project at line 4051) unmounts every row inside it, including the thread that is currently open. Everywhere else in this list the route thread is rescued from collapse — renderedSettledThreads keeps the open row when the Settled shelf is collapsed, and LegacySidebar does the same via pinnedCollapsedThread — so its highlight and row affordances stay reachable.
Consider keeping the route thread's row rendered when its platform/project group is collapsed, matching the existing shelf behavior.
Posted via Macroscope — UI Consistency
…gdotgg#7160) Co-authored-by: Cursor <cursoragent@cursor.com>
This reverts commit 44275cd (upstream PR pingdotgg#7160). Removes the Hermes provider, local chat importers, and the sidebar's group-threads-by-provider UI. Kept the ACP auth-method resolution (resolveAcpAuthMethodId / skipAuthenticate) that the Cursor provider work depends on, and the queued-turn orchestration events that landed alongside it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note 🤖 GPT-5.6 Sol responding on behalf of Theo Closing this PR after an automated pass over open pull requests. Adds a broad thread-import or state-transfer system that overlaps trusted work in #5499. |
What Changed
Why
UI Changes
Checklist
Note
Medium Risk
Large surface area across orchestration events, persistence projections, and child-process ACP sessions; imports mutate many threads/projects and depend on local CLI/DB paths, but RPC is gated to operate scope and idempotency is handled via existing bindings.
Overview
Adds Hermes Agent as a first-class provider (driver, ACP adapter, health snapshots, GitHub release updates) and documents it in the README.
Chat import is wired end-to-end: new orchestration commands
thread.message.import/thread.history.importemitthread.history-imported(and taggedthread.message-sent) withimportedHistoryso checkpoints and projections treat bulk history differently from live turns. WebSocket RPCserverImportHermesSessionsandserverImportLocalChats(operate scope) run exporters/parsers that create projects/threads, bulk-import messages and tool activities, and record provider session bindings for resume.ACP runtime gains Hermes-oriented behavior: optional non-JSON stdout stripping, configurable
session/newtimeout, and auth method resolution from initialize when not overridden. Provider maintenance can resolve latest versions from GitHub releases (used for Hermes). Hermes text generation helpers are stubbed as unsupported.Reviewed by Cursor Bugbot for commit b04ceae. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add multi-provider chat import for Hermes, Codex, and OpenCode with sidebar platform grouping
hermes --version.thread.message.importandthread.history.importorchestration commands that emit new events (thread.history-imported), persisted through the projector and projection pipeline.serverImportHermesSessionsandserverImportLocalChatsWS RPC endpoints (requiring orchestration operate scope) wired to import flows triggered from the settings panel.thread.message-sentevents skip pre-turn baseline creation in the checkpoint reactor, which is a behavioral change for any downstream logic that relied on baseline consistency.Macroscope summarized b04ceae.