feat(provider): port Cursor ACP support and unify provider system#63
Merged
Conversation
Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: codex <codex@users.noreply.github.com> Co-authored-by: Julius Marminge <julius@macmini.local> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: cursor[bot] <206951365+cursor[bot]@users.noreply.github.com>
…#2218) Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Post-cherry-pick adjustments so typecheck passes across all 9 packages:
- `providerIconUtils.ts`: add `cursor: CursorIcon` to `PROVIDER_ICON_BY_PROVIDER`
(discriminated-union now has four providers).
- `ChatView.tsx`: extend `modelOptionsByProvider` memo with `opencode` and
`cursor` buckets, and use `createModelSelection()` for `selectedModelSelection`
so `exactOptionalPropertyTypes` narrows correctly through the new 4-arm union.
- `store.ts`: split `ProviderKind` out of the `import type {}` block because
`toLegacyProvider` uses it as a runtime value via `Schema.is(ProviderKind)`.
- `CompactComposerControlsMenu.browser.tsx`: drop the "can hide the interaction
mode section" test — it exercises upstream's `showInteractionModeToggle` /
`runtimeMode` / `onRuntimeModeChange` props that MarCode's menu doesn't
accept.
- `TraitsPicker.browser.tsx`: add `jiraTaskContexts: []`, `quotedContexts: []`
to the Cursor/OpenCode test draft states (MarCode's
`ComposerThreadDraftState` requires them).
- `ProviderCommandReactor.ts`: guard against `sendTurnRequest.value` being
`undefined` since `buildSendTurnRequestForThread` can early-return `void`.
- `OpenCodeAdapter.test.ts`: add missing `remove: () => Effect.void` to the
`ProviderSessionDirectory` test double (shape added by OpenCode refactor).
- `ProviderService.test.ts`: use existing `AnalyticsServiceNoopLive` instead
of a non-existent `AnalyticsService.layerTest`.
Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
…duplicate Build/Plan toggle
**Activity card routing for Cursor/OpenCode**
Cursor ACP and OpenCode tool invocations were falling through to `dynamic_tool_call`
(the generic "work" batch) because each provider had its own incomplete classifier
hardcoded:
- `AcpCoreRuntimeEvents.canonicalItemTypeFromAcpToolKind` missed `read`, and
incorrectly mapped `fetch` → `web_search` (should be `web_fetch`).
- `AcpRuntimeModel.canonicalItemTypeFromAcpToolKind` was a duplicate of the above.
- `OpenCodeAdapter.toToolLifecycleItemType` had no `file_read` branch (read/ls/
glob/grep all fell through), conflated `web*` into `web_search`, and used naive
substring matching.
Introduce a single shared classifier in `@marcode/shared/toolActivity`:
`classifyToolLifecycleItemType({ toolName, kind, title })`. Deterministic
token-based matching over the full `TOOL_LIFECYCLE_ITEM_TYPES` vocabulary.
All three provider classifiers now delegate to it, so any new provider just
feeds hints in and gets the correct `itemType` — no touching the web
`MessagesTimeline` router, whose existing predicates already dispatch
correctly by `itemType`.
Added 12 unit tests covering MCP prefix, bash/terminal/run-command, read/view/cat,
ls/glob/grep/codebase_search, edit/write/patch/multiedit, web_fetch vs web_search,
subagent/task, image_view, ACP kind literals (execute/read/edit/delete/move/
search/fetch), title fallback, dynamic_tool_call default, and toolName > kind
priority.
**Duplicate Build/Plan selector**
`composerProviderRegistry` already defines `showInteractionModeToggle: false`
for OpenCode (whose agent system has its own Build/Plan/Chat modes), but
`ChatView.tsx` rendered an inline Plan/Build `BotIcon` Button at lines ~5149
that ignored this flag. Gate the button by
`composerProviderControls.showInteractionModeToggle`. Import
`getComposerProviderControls` from the registry and memoize per
`selectedProvider`. Matches upstream's extracted `ComposerFooterModeControls`
behavior (which MarCode has the file for but doesn't render from `ChatView`).
…bugs for Cursor/OpenCode
**1. Command text now shows in the bash card for Cursor/OpenCode**
OpenCodeAdapter emitted `data: { tool, state }` which buried the command
under `state.input.command` — the client's extractToolCommand only descends
into `data.command` / `data.input.command`, so the card showed output-only.
Rename the OpenCode payload shape to match what client extractors expect:
data: {
toolName: part.tool, // for extractToolName → ExplorationCard heading
input: part.state.input, // for extractToolInput → path/query extraction
command: part.state.input?.command, // for extractToolCommand
tool, state, // preserved for debugging
}
ACP already sets `data.command` via extractToolCallCommand, but the payload
also needed `data.toolName` and `data.input` aliases so the same ExplorationCard
per-tool heading logic (`lower === "read"`, `lower === "grep"`, etc.) fires
for Cursor. Added both in AcpRuntimeModel.makeToolCallState.
**2. Exploration card no longer renders raw XML for Cursor/OpenCode**
`classifyToolAction` in shared/toolActivity.ts used ad-hoc `kind === "read"` /
`title === "read file"` string matching. If Cursor's ACP payload had
`kind: "other"` with a long XML-ish title like
`Read: <path>/foo</path><type>file</type><content>…`, the classifier fell
into the `"other"` branch and deriveToolActivityPresentation preserved the
raw title verbatim. Make it itemType-first: trust the canonical itemType
set upstream by `classifyToolLifecycleItemType` (`file_read`,
`command_execution`, `file_change`, `web_search`, `web_fetch`) and map to
the presentation action directly. Falls back to `kind`/`title` heuristics
only when itemType is missing.
**3. Turn-finish sound no longer fires on thread create, plays on
follow-up, and isn't doubled on real completion**
Three root causes, one coherent fix:
- *Spurious sound on thread create*: the `turnWasActive` guard used
`thread.session?.activeTurnId !== undefined`, but `null !== undefined`
is `true` — Cursor/OpenCode session.started emits session objects with
`activeTurnId: null`, which was being mis-read as "has active turn" and
firing a bogus turn-completed notification before any turn ran. Change
to `!= null`.
- *No sound on follow-up turn end*: the shell stream can race ahead of the
detail stream, writing `thread.session.status = "ready"` to the store
before the detail stream's thread.session-set event reaches derivation.
The previous batch-local `threadTransitionedToRunning` check doesn't help
across batches. Add persistent module-scope `threadsWithActiveTurn` set:
armed when session-set arrives with status=running, cleared when
completion fires.
- *Doubled sound on turn end*: applyEnvironmentThreadDetailEvent wraps each
single event in its own batch, so `thread.session-set` (ready) and
`thread.turn-diff-completed` fired separate notifications — the batch-local
`completionFiredThreadIds` only deduped within a single batch. Add
persistent `recentlyFiredCompletion` map with a 3s TTL so the fallback
path collapses into the primary for the same real turn end.
Added three regression tests covering each bug, plus a test-only
`__resetTurnNotificationStateForTests` export so existing tests can
isolate module-scope state between cases.
Also updated AcpRuntimeModel.test.ts to assert the new toolName/input
aliases in the tool-call data payload.
…fetch URL
**ExplorationCard: readable headings for Cursor/OpenCode/generic tool names**
The previous heading dispatcher only matched exact toolName strings from
Claude's vocabulary ("Read", "Grep", "Glob", "List", "Find"). Cursor sends
`kind` values ("read", "search") and OpenCode ships snake_case tool names
(`codebase_search`, `list_directory`, `view_file`). Both fell through to the
`${titled}: ${summary}` fallback, which printed the raw tool output verbatim
— that's what surfaced the ugly
`Read: <path>/Users/.../foo.ts</path> <type>file</type> <content>…` rows.
Changes in `ExplorationCard.tsx`:
- Expand `READ_TOOL_NAMES` (`view_file`, `read_file`) and `SEARCH_TOOL_NAMES`
(`list_directory`, `codebase_search`, `file_search`, `tree`, `ripgrep`,
`rg`) to recognize the vocabulary every provider actually uses.
- Add `inputFilePath` fallbacks: `file`, `target_file`, `targetFile`,
`directory`, `dir`.
- Route specific tools (`read_file`, `view_file`, `ripgrep`, `rg`,
`list_directory`, `codebase_search`, `file_search`, `tree`) to the right
branch; drop the old colon-separated `${titled}: ${summary}` fallback that
produced the garbage.
- Final fallback now routes by canonical `itemType` (`file_read`) as a
provider-agnostic safety net so Cursor ACP `kind: "read"` without a
specific toolName still prints cleanly.
- Parse `<path>/foo</path>` out of Cursor/Codex XML-wrapped detail so the
heading extracts the real path when structured `toolInput` is missing.
**Relative paths in row headings**
Added `shortenPath(absolutePath)` with a two-pass strategy:
1. Start from a known project-root marker (`apps/`, `packages/`, `src/`,
`lib/`, `components/`, `modules/`) when the absolute path contains one.
2. Fall back to the last three path segments.
So `/Users/tyulyukov/WebstormProjects/personal/marcode/apps/web/src/components/chat/MessagesTimeline.tsx`
now renders as `apps/web/src/components/chat/MessagesTimeline.tsx`.
**Hover tooltip on each exploration row**
Wraps each row in `<Tooltip>`. The body shows the full absolute path +
structured hints (pattern, search query) + a cleaned version of the detail
(with the `<content>…` trailer stripped — the tooltip was otherwise drowned
in truncated file contents). Hidden entirely when there's nothing
meaningful beyond the heading.
**WebFetchCard: extract URL from more sources**
`deriveUrl` was strict: only `input.url` or a `detail` starting with
`https://`. Cursor's ACP tool shapes put the URL under `uri`/`link`/
`target`/etc., and detail often arrives as `<url>https://…</url>` XML or
prefixed by the tool response. Now:
- Accept `url | uri | link | target | href | endpoint | address` in input.
- Match `<url>…</url>`/`<uri>…</uri>`/`<link>…</link>` XML in detail.
- Fallback to the first bare `https://` token anywhere in detail.
**Notification suite green** — no changes; the 3-test regression guard from
the prior commit still covers the Cursor/OpenCode sound paths.
…ange for Cursor/OpenCode
Builds on the ACP/Cursor/OpenCode tool catalog I mapped from the official
docs (agentclientprotocol.com/protocol/tool-calls, opencode.ai/docs/tools)
to route every documented tool into the right MarCode card without
scattering provider-specific logic across the UI.
**TodoWrite → plan/tasks sidebar**
Claude has always translated its `TodoWrite` tool invocations into
`turn.plan.updated` runtime events so the plan projector + plan sidebar
track agent todos. Cursor (via ACP) and OpenCode both surface todos too —
Cursor's agent can emit a `tool_call` titled "TodoWrite" with
`rawInput: { todos: [...] }`; OpenCode's `todowrite` builtin has the same
shape. Previously neither reached the plan sidebar.
- Lifted Claude's `isTodoTool` + `extractPlanStepsFromTodoInput` into
`@marcode/shared/toolActivity` as `isTodoWriteTool` +
`extractPlanStepsFromTodos`. Lenient matcher (casing / `_`/`-`/`.`
separators / `todos` alias) covers every variant.
- Accepts ACP's `in_progress`/`inProgress`/`in-progress` + `done` status
aliases so it normalizes cleanly whichever dialect the agent sends.
- `AcpRuntimeModel.parseSessionUpdate` now also emits `PlanUpdated` when
a tool_call/tool_call_update is a TodoWrite (in addition to the existing
native `sessionUpdate: "plan"` path that Cursor uses directly).
- `OpenCodeAdapter` emits `turn.plan.updated` right after the tool
runtime event when `part.tool` matches `isTodoWriteTool`.
- `ClaudeAdapter` now delegates to the shared helper instead of its
private copy (behavior preserved, verified by existing tests).
**apply_patch → FileChangeCard**
OpenCode's `apply_patch` tool (per opencode.ai/docs/tools) ships the full
patch as `patchText` with `*** Update File: <path>` / `*** Add File` /
`*** Delete File` marker lines — Codex and Cursor follow the same OpenAI
apply-patch convention. `inlineDiff.extractDiffPreviews` only knew Claude's
`{ old_string, new_string, file_path }` shape, so patch calls produced no
preview and fell through to a generic card.
- Added `PATCH_TOOL_NAMES` set (`apply_patch`, `ApplyPatch`, `applyPatch`,
`patch`, `Patch`, `apply-patch`) and an `extractApplyPatchHunks` parser
that scans the marker-based format, handles multi-file patches,
distinguishes Update/Add/Delete, and builds unified-diff lines for the
existing `FileChangeCard`.
- Broadened `EDIT_TOOL_NAMES` (`str_replace`, `search_replace`, PascalCase
variants) and `WRITE_TOOL_NAMES` (`create`) to cover Cursor's and
OpenCode's alternate naming.
**Tests**
- `packages/shared/src/toolActivity.test.ts`: +7 cases covering
`isTodoWriteTool` matching, alias/status normalization in
`extractPlanStepsFromTodos`, and edge cases (null, empty, non-object
entries, activeForm fallback).
- `apps/web/src/lib/inlineDiff.test.ts`: +5 cases for apply_patch —
Update File hunk → edit preview, Add File → write preview, multi-file
patches, alternate input keys (`patch`, `diff`), bash fallthrough.
- Existing regression guards (service.notification-wiring,
store.test, ProviderRuntimeIngestion, all provider adapter tests)
still pass.
…served a running session-set event first
The previous implementation kept three fallback heuristics for cases where
stored session state might lag the derivation pass:
- thread.session?.orchestrationStatus === "running"
- thread.session?.activeTurnId != null
- thread.latestTurn?.state === "running"
Any of those evaluating true would let a `thread.session-set status=ready`
event fire a turn-completed notification. The OpenCode first-message flow
exposes a bug in that: the shell stream's thread-upserted can write
`session: { status: "running" | "ready", activeTurnId: <turnId> }` to the
store BEFORE the detail stream's session.started / turn.started
`thread.session-set` events reach `deriveTurnNotificationTriggers`. When
the next detail event arrives (anywhere from session.state.changed to
thread.started to turn.completed), the stored state reads like a real turn
ran, so the notification fires — prematurely, at every first message, no
matter how many times we massage the activeTurnId/null checks.
Drop the three fallbacks entirely. The only signal allowed to gate a
completion is the persistent `threadsWithActiveTurn` flag, which is armed
exclusively by a `thread.session-set` event with `status: "running"`. If
the client never observed that event, no sound plays — full stop. Same
gate now applies to the `thread.turn-diff-completed` fallback path so the
checkpoint echo can't sneak around it either.
Trade-off: a truly edge-case reconnect-mid-turn scenario (client connects
after status=running was already delivered in a snapshot and will never
re-fire as an event) will NOT get a completion sound. That is strictly
preferable to the current symptom — a user-reported spurious sound on
EVERY first message of EVERY OpenCode thread.
**Test updates**
- "fires turn-completed when the thread has an active latest turn..." →
flipped to assert NO fire (the stored-state-alone path is intentionally
gone).
- "fires turn-completed when the session tracks an activeTurnId even
without running status" → same flip.
- Tests that relied on `makeThread()`'s default orchestrationStatus="running"
to fire now explicitly arm the flag with a prior running session-set
event (matches real-world event ordering).
- Added a full OpenCode-first-message regression test: session.started
(ready) → thread.started (ready) → turn.started (running) →
turn.completed (ready) across four batches, asserting exactly ONE fire
at the final batch and silence for every earlier one.
All 22 notification tests green, 9 packages typecheck clean, store +
service.notification-wiring regression guards still pass.
NOTE TO USER: if the sound STILL fires on first message after this lands,
your dev server / electron process is serving stale JS. Restart the dev
server (kill `bun run dev` / electron and re-run) and hard-reload the
renderer (⌘⇧R).
…w provider blob
Previous tooltip body concatenated the absolute path + `pattern: <x>` +
a "cleaned" detail (with the `<content>…` trailer stripped). That was way
too much — and when the cleaned detail still had junk, users saw a wall
of raw provider payload on hover.
The intent was simpler: let the user read the full inline heading when
the row truncates it (narrow sidebar layouts, long paths). So the
tooltip now shows the SAME heading string as the row, just with the
full absolute path instead of the project-rooted shortened form.
Implementation: `explorationEntryHeading` gained an optional
`{ pathMode: "short" | "full" }` param. Row renders `short`, tooltip
renders `full`. If both happen to produce the same string (tool had no
path), we skip the tooltip entirely.
Also dropped `whitespace-pre-wrap` from the TooltipPopup since the body
is now a single line — `break-all` is enough.
… raw payload.detail
CI surfaced four session-logic.test.ts failures after my recent edits to
`toDerivedWorkLogEntry`. The regression was that I'd bypassed the smart
`extractToolDetail` helper and just took the raw `payload.detail` string,
which meant:
- `title: "Read File"` + `detail: "Read File"` → `entry.detail = "Read File"`
(duplicate, should be suppressed).
- `title: "grep"` + `detail: "grep"` + `rawOutput: { totalFiles: 19 }` →
`entry.detail = "grep"` (should be `"19 files"` from the rawOutput summary).
- Read-file completion with `rawOutput.content` → detail stayed `"Read File"`
instead of the first-line preview.
Restore `extractToolDetail` on the fallback path. That helper does
normalized-equivalence dedup (title vs detail) AND falls back to
`summarizeToolRawOutput` (which handles `totalFiles` / `content` / `stdout`).
Exit-code extraction is split off into a separate pass so command cards
still badge pass/fail regardless of which detail source won.
CI verification: all 1074 web tests + 995 server tests + 119 shared tests
now pass. The 4 previously-failing tests all depend on the equivalence
dedup + rawOutput summarization this restores.
CI browser-test failure: `GeneralSettingsPanel observability > shows an OpenCode server URL field in provider settings` crashed with `No QueryClient set, use QueryClientProvider to set one`. The test renders `<GeneralSettingsPanel>` which nests `JiraSettingsSection` (calls `useQueryClient`), but this particular case was the only one in the file that skipped the `QueryClientProvider` wrapper its siblings all use. Add the wrapper, matching the "Open logs folder" test directly above it.
…orphaned-session stop **1. One card per file for multi-file patches** Multi-file tool calls (apply_patch with N file markers, Claude MultiEdit, etc.) previously rendered as a single collapsed "N files changed — Show full diff" card. That UX forced the user to expand the aggregate just to see any one file's diff. `MessagesTimeline.logic.ts` now splits a file-change entry with N `diffPreviews` hunks into N individual file-change rows. Each row reuses the existing FileChangeCard single-hunk render path, so users see inline diff previews for every file without expanding. Row ids are suffixed with `#<index>` to keep each row's identity stable across updates. **2. Stop button no longer deadlocks on orphaned provider sessions** User hit this repro: thread shows "Working" for hours, clicking the red stop button emits `ProviderValidationError: Cannot recover thread '<id>' because no provider resume state is persisted` into the logs, and the UI stays stuck. Root cause: - MarCode's `stopSession` deletes the provider binding (commit 8940163, diverges from upstream pingdotgg#2125) — so after a crash/restart the binding can exist with status="stopped" or with resumeCursor cleared. - `ProviderService.interruptTurn` used `allowRecovery: true`, which tried to RESURRECT the dead session via `recoverSessionForThread` just to kill it. If resume state is missing → validation error bubbles out → interrupt fails → thread.session.status stays "running" → UI stays "Working". Fixes in two layers: - `ProviderService.interruptTurn` now uses `allowRecovery: false` and skips the adapter call when `isActive === false`. You can't interrupt a session that doesn't exist; there's nothing to kill. - `ProviderCommandReactor.processTurnInterruptRequested` wraps the provider interrupt in `Effect.catchCause`. If the provider call fails (or, now more rarely, the adapter is no-op because recovery is off), the reactor dispatches a `thread.session.set` with status="interrupted" and `activeTurnId: null`, so the UI unblocks from "Working" regardless of what the provider does. Also appends a visible failure activity explaining "session was orphaned; force-stopped. Send a new message to resume." **Verification** - Typecheck: all 9 packages clean. - @marcode/web: 1074 tests pass (incl. 14 MessagesTimeline.logic). - @marcode/server: 995 pass, 5 skipped (incl. 24 ProviderCommandReactor, 26 ProviderService). - Browser: 161 pass, 6 skipped.
…n so reactor force-clears Follow-up to 5a63be9. The previous change silenced the error but also broke the reactor's catchCause path: `interruptTurn` would return successfully when no active session existed (isActive === false), which meant the reactor NEVER ran its force-clear code, and the "Working" indicator stayed stuck on any thread whose provider session was orphaned between restarts. Raise a specific `ProviderValidationError` from `interruptTurn` when the routed session is inactive, so the reactor's existing catchCause handler always runs, always dispatches a `thread.session.set` with status="interrupted", and always surfaces the visible "Turn force-stopped" activity. The contract's `Effect<void, ProviderServiceError>` return type is preserved — the failure is exactly what the reactor already knows how to handle. User-reported repro: thread `013848ab-2305-4125-92b9-9ed3c6faeffe` stuck on "Working" for hours; clicking Stop now correctly transitions the thread to an interrupted/error-free state so the user can send a new message to re-establish the provider session. Verified: - Typecheck: 9/9 packages clean - @marcode/server: 995 pass, 5 skipped (24 ProviderCommandReactor, 26 ProviderService). - @marcode/web: 1074 pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing