feat: harden Cursor ACP session, wire extensions + config, add protocol tooling - #48
Merged
Conversation
…tensions, add protocol tooling Remove old CursorSession (~1000 LOC) and make AcpSession the sole Cursor adapter. Wire MCP server pass-through, cursor extension handlers (ask_question, create_plan, update_todos), and set_config through all 5 stack layers (GenServer → SessionManager → Channel → ClientManager → ProviderAdapterShape). Key changes: - AcpSession: command parsing/storage, cursor extension handlers, set_config RPC, error detail surfacing (Zod data field), capability gating on agentCapabilities.loadSession, tool_items Map for kind lookup - Contracts: ProviderCommand schema, session.commands.available event type, supportsUserInput: true, sessionModelSwitch: "in-session" for Cursor - Server: turn/plan/created + session/commands_available event mapping, session.configured preserves currentModeId, MCP translation for ACP - Web: ProviderCommandsPanel with fuzzy search + a11y, deriveProviderCommands from activities, Commands toolbar button for Cursor sessions - Tooling: scripts/acp-probe.ts interactive CLI for ACP protocol probing, 2 new wire capture fixtures (composer2, gpt54mini reasoning + cancel) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughMigrates Cursor from stdio stream-json to ACP JSON‑RPC (AcpSession), removes CursorSession and its tests/flags, adds set_config APIs across harness/server layers, surfaces provider command discovery to the frontend, rebrands desktop to T3 Code‑OTP, adds ACP probe tooling and MCP wiring. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Web as Web Client
participant Server as Server (HarnessAdapter)
participant Manager as HarnessClientManager
participant Harness as Harness (SessionManager)
participant ACP as AcpSession
Web->>Server: setConfig(threadId, configId, value)
Server->>Manager: setConfig(threadId, configId, value)
Manager->>Harness: Phoenix "session.setConfig" request
Harness->>ACP: SessionManager.set_config(threadId, configId, value)
ACP->>ACP: Perform RPC (session/set_config_option) and update state
ACP->>Harness: emit session/configured (with configOptions)
Harness->>Manager: Phoenix reply
Manager->>Server: resolve promise
Server->>Web: return success
sequenceDiagram
autonumber
participant Agent as Cursor Agent
participant ACP as AcpSession
participant Harness as Harness (SessionManager)
participant Server as Server (Runtime)
participant Web as Web Client
Agent->>ACP: notify availableCommands
ACP->>ACP: parse availableCommands → payload.commands
ACP->>Harness: emit session/commands_available
Harness->>Server: provider runtime event (session.commands.available)
Server->>Web: ingest activity (session-commands)
Web->>Web: deriveProviderCommands() → show ProviderCommandsPanel
Web->>Web: user picks command → insert into composer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
size:XXL
vouch:trusted
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (1)
apps/server/src/provider/mcpTranslation.ts (1)
114-138: Use an explicit ACP MCP payload type instead ofRecord<string, unknown>[]This payload crosses process boundaries; typing it as a concrete union will catch shape drift earlier.
♻️ Suggested typing refactor
+type AcpMcpServer = + | { + readonly name: string; + readonly type: "command"; + readonly command: string | undefined; + readonly args: ReadonlyArray<string>; + readonly env: ReadonlyArray<{ readonly key: string; readonly value: string }>; + } + | { + readonly name: string; + readonly type: Exclude<McpServerConfig["transport"], "stdio">; + readonly url: string | undefined; + readonly headers: ReadonlyArray<{ readonly key: string; readonly value: string }>; + }; + -export function acpMcpServersFromResolved(config: ResolvedMcpConfig): Record<string, unknown>[] { +export function acpMcpServersFromResolved(config: ResolvedMcpConfig): AcpMcpServer[] {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/provider/mcpTranslation.ts` around lines 114 - 138, The function acpMcpServersFromResolved currently returns a loosely typed Record<string, unknown>[] which can hide schema drift across process boundaries; create a concrete union payload type (e.g., AcpMcpServer = StdioServerPayload | TransportServerPayload) plus a KeyValuePair type for env/headers, then update the function signature to return AcpMcpServer[] and ensure each branch returns the correct discriminated shape (stdio branch returns the StdioServerPayload with command, args, env: KeyValuePair[]; other branch returns TransportServerPayload with type set to transport, url, headers: KeyValuePair[]). Also export or reuse these types where the payload is consumed so the IPC contract is typed end-to-end.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main.ts`:
- Around line 60-68: LEGACY_USER_DATA_DIR_NAME currently uses the new "T3
Code-OTP ..." name so resolveUserDataPath() won't detect existing Chromium
profiles; change LEGACY_USER_DATA_DIR_NAME to the original Electron profile name
(use "T3 Code (Dev)" for dev and "T3 Code (Alpha)" for prod) so the constant
matches legacy directories and upgrades preserve user state—update the
LEGACY_USER_DATA_DIR_NAME definition near BASE_DIR / resolveUserDataPath()
accordingly.
In `@apps/harness/config/runtime.exs`:
- Around line 14-15: Restore the removed runtime toggle by adding back a harness
config key named cursor_acp_enabled (e.g., config :harness, cursor_acp_enabled:
System.get_env("CURSOR_ACP_ENABLED") |> parse_bool_or_default(true)) alongside
the existing harness_secret so CursorSession fallback/kill-switch remains
available; reference the config block where harness_secret is set and ensure
CursorSession code reads :harness, :cursor_acp_enabled to toggle ACP routing at
runtime.
In `@apps/harness/lib/harness/dev/explain.ex`:
- Around line 30-32: The "Cursor" description in the comparison block is
inconsistent: update the line that currently describes Cursor as "stdout
stream-json" / per-turn to instead indicate that Cursor uses bidirectional stdio
(ACP/JSON-RPC) like Codex; locate the lines containing the strings "Cursor",
"Codex", and "Claude" in apps/harness/lib/harness/dev/explain.ex and change the
Cursor row text to match the ACP/JSON-RPC wording used for Codex so the block is
internally consistent.
In `@apps/harness/lib/harness/providers/acp_session.ex`:
- Around line 520-524: The handler for the pattern %{from: from, timer: timer,
method: "session/set_config_option"} updates state.config_options and replies
via GenServer.reply but does not emit the runtime signal; after updating
config_options (and before returning {:continue, ...}) call the existing emitter
path (e.g., invoke current_mode_update or the function that broadcasts
"session/configured") with the new state so the upstream
Layers/HarnessClientManager sees the change; ensure you pass the updated state
(with config_options set) into current_mode_update so the session/configured
event is emitted.
- Around line 646-663: The "cursor/create_plan" branch in handle_rpc_request
currently calls put_provider_pending and leaves an orphaned pending entry in
state.pending with no handler to resolve it; either make this request
fire-and-forget by not storing it as pending or add an explicit response path to
clear it. Fix by changing the "cursor/create_plan" handling in
handle_rpc_request: if the intent is fire-and-for-get, remove the
put_provider_pending call (or immediately mark it acknowledged) and emit the
notification (emit_event) as now; otherwise implement a corresponding
handle_call/handle_cast branch that matches method "cursor/create_plan" (or a
resolver function) to send the JSON-RPC response and remove the pending entry
from state.pending (using the same request id semantics), ensuring the pending
map is always cleaned on session exit or provider shutdown to avoid orphaned
waiters.
- Around line 628-644: The cursor/ask_question requests created in
handle_rpc_request (which calls put_provider_pending with "cursor/ask_question")
are not being closed as user-input on teardown; update reject_all_pending/2 to
special-case entries created with "cursor/ask_question" (or detect pending
entries whose original RPC was "cursor/ask_question") and emit a
"user-input/resolved" (or equivalent user-input cancellation) event for those
pending items instead of the generic request/resolved path so downstream
user-input waiters are notified and not orphaned; ensure the matching uses the
same identifying string ("cursor/ask_question") produced by handle_rpc_request
and that the payload includes the original requestId/params as
put_provider_pending created.
- Around line 32-40: The diagnostics path still calls MapSet.size/1 on
state.tool_items even though tool_items was changed to a plain map; update the
:get_diagnostics logic to call map_size(state.tool_items) instead of
MapSet.size(state.tool_items) (locate the call in the function handling
:get_diagnostics, referencing state and tool_items) so diagnostics no longer
crash when tool_items is a map.
In `@apps/harness/lib/harness/session_manager.ex`:
- Line 407: The mapping for "cursor" currently returns AcpSession directly;
change provider_module/1 so "cursor" maps to CursorSession (or implements a
prioritized lookup that prefers AcpSession but falls back to CursorSession) to
retain the legacy fallback path; update provider_module("cursor") to return
{:ok, CursorSession} or implement logic in provider_module/1 to attempt {:ok,
AcpSession} then fall back to {:ok, CursorSession} so CursorSession remains
available until ACP is proven end-to-end.
- Around line 177-184: The set_config/3 path uses with_session/2 but does not
guard against GenServer.call exits so a dying/slow session will crash the
channel; update set_config/3 to wrap the session call in a try/catch like the
pattern in with_mcp_session/2 (lines around with_mcp_session/2) to catch :exit
and return {:error, :session_unavailable} (or similar) instead of letting the
exit propagate, or refactor by adding a shared exit-safe helper (e.g.,
safe_with_session/2) that encapsulates the try/catch and reuse it from
set_config/3 and other RPC entry points to ensure all session calls handle
GenServer.call exits safely.
In `@apps/server/src/os-jank.ts`:
- Line 33: The change to return join(OS.homedir(), ".t3-otp") will orphan legacy
state in ~/.t3, so update the logic that computes the base dir (where join and
OS.homedir() are used) to detect an existing legacy directory join(OS.homedir(),
".t3") and either (a) prefer the legacy dir if it exists, or (b) perform a
one-time migration copying/moving known state files (telemetry ID and global MCP
config) from ".t3" into ".t3-otp" and preserve file permissions, then remove or
mark the legacy files; ensure this logic references the exact return point (the
function that currently returns join(OS.homedir(), ".t3-otp")) and handles
errors/logging so telemetry and MCP config survive upgrades.
In `@apps/server/src/provider/Layers/HarnessClientAdapter.ts`:
- Around line 937-962: The case handling event.method === "turn/plan/created" is
unreachable because mapHarnessEventToRuntimeEvents returns earlier for
event.kind === "request"; move the "turn/plan/created" branch so it executes
before the generic request handling (or add a special-case inside the request
branch) so it won't be swallowed by the request.opened logic; update
HarnessClientAdapter.ts within mapHarnessEventToRuntimeEvents to check for
event.method "turn/plan/created" (or detect payload.plan within the event.kind
=== "request" branch) and then produce the turn.plan.updated runtime event as
currently implemented.
In `@apps/web/src/components/chat/TraitsPicker.tsx`:
- Around line 287-289: The early-return in TraitsPicker that checks "if (effort
=== null && thinkingEnabled === null) return null" hides controls for models
that only support Fast Mode; update this guard to also consider
"caps.supportsFastMode" (i.e., return only when effort, thinkingEnabled, and
caps.supportsFastMode are all null/false) so Fast Mode controls remain visible;
apply the same change to the analogous guard in TraitsMenuContent so trigger and
content visibility remain consistent (use the same combined condition
referencing effort, thinkingEnabled, and caps.supportsFastMode).
In `@apps/web/src/session-logic.ts`:
- Around line 185-205: The filter in deriveProviderCommands currently only
checks for a string name and casts to ProviderCommand; update it to validate the
full ProviderCommand shape before casting by ensuring the candidate is a
non-null object and has name, description, and type properties of the expected
types (e.g., typeof name === "string", typeof description === "string", typeof
type === "string" or check allowed type values if applicable), then return only
those that pass so malformed persisted/runtime payloads are excluded from the
returned ProviderCommand[].
In `@scripts/acp-probe.ts`:
- Around line 372-411: The probe currently mutates session state and skips the
real config path: replace the `/mode` branch to call the config API path used in
production (use sendRequest with "session/set_config_option" instead of
"session/set_mode") and stop using the destructive placeholder in `/config`
(remove the hardcoded value "default[]" and instead read from a local cached
configOptions that is populated from "session/new" and any
"session/config_updated" notifications); keep using sendRequest/sendNotification
but update the code references to configOptions and session/set_config_option so
`/mode` changes are exercised via the same config option flow as `/config`
without mutating the session unexpectedly.
- Around line 470-473: In the "/quit" (and "/q") case replace the direct
process.exit(0) call with rl.close() so the existing rl.on("close") cleanup
handler runs and can kill the spawned cursor ACP child process; if you prefer
explicit shutdown, explicitly kill the spawned child (the cursor ACP child
process created earlier) and then call rl.close() instead of process.exit(0) to
ensure proper cleanup.
In `@scripts/build-desktop-artifact.ts`:
- Around line 453-455: The artifactName change to
"T3-Code-OTP-${version}-${arch}.${ext}" breaks the smoke tests in
scripts/release-smoke.ts which still expect "T3-Code-*"; update the
assertions/fixtures in scripts/release-smoke.ts to match the new artifact naming
(search for checks referencing "T3-Code-" or hardcoded artifact names) and
ensure any helper/constant used for expected artifactName is updated or
centralized so both the build script (artifactName) and release-smoke.ts use the
same expected pattern.
---
Nitpick comments:
In `@apps/server/src/provider/mcpTranslation.ts`:
- Around line 114-138: The function acpMcpServersFromResolved currently returns
a loosely typed Record<string, unknown>[] which can hide schema drift across
process boundaries; create a concrete union payload type (e.g., AcpMcpServer =
StdioServerPayload | TransportServerPayload) plus a KeyValuePair type for
env/headers, then update the function signature to return AcpMcpServer[] and
ensure each branch returns the correct discriminated shape (stdio branch returns
the StdioServerPayload with command, args, env: KeyValuePair[]; other branch
returns TransportServerPayload with type set to transport, url, headers:
KeyValuePair[]). Also export or reuse these types where the payload is consumed
so the IPC contract is typed end-to-end.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bce7ead0-ad05-40af-aaef-9153568e43a1
📒 Files selected for processing (29)
AGENTS.mdREADME.mdapps/desktop/package.jsonapps/desktop/src/main.tsapps/harness/config/runtime.exsapps/harness/lib/harness/dev/explain.exapps/harness/lib/harness/providers/acp_session.exapps/harness/lib/harness/providers/cursor_session.exapps/harness/lib/harness/providers/provider_behaviour.exapps/harness/lib/harness/session_manager.exapps/harness/lib/harness_web/harness_channel.exapps/harness/test/fixtures/acp/composer2_probe_real.ndjsonapps/harness/test/fixtures/acp/gpt54mini_reasoning_real.ndjsonapps/harness/test/harness/providers/cursor_session_test.exsapps/server/src/orchestration/Layers/ProviderRuntimeIngestion.tsapps/server/src/os-jank.tsapps/server/src/provider/Layers/HarnessClientAdapter.tsapps/server/src/provider/Layers/HarnessClientManager.tsapps/server/src/provider/Services/HarnessClientAdapter.tsapps/server/src/provider/mcpTranslation.tsapps/web/src/components/ChatView.tsxapps/web/src/components/chat/ProviderCommandsPanel.tsxapps/web/src/components/chat/TraitsPicker.tsxapps/web/src/session-logic.tsdocs/index.htmlpackages/contracts/src/orchestration.tspackages/contracts/src/providerRuntime.tsscripts/acp-probe.tsscripts/build-desktop-artifact.ts
💤 Files with no reviewable changes (2)
- apps/harness/test/harness/providers/cursor_session_test.exs
- apps/harness/lib/harness/providers/cursor_session.ex
👮 Files not reviewed due to content moderation or server errors (6)
- apps/harness/lib/harness_web/harness_channel.ex
- apps/web/src/components/ChatView.tsx
- apps/harness/test/fixtures/acp/gpt54mini_reasoning_real.ndjson
- apps/harness/test/fixtures/acp/composer2_probe_real.ndjson
- packages/contracts/src/providerRuntime.ts
- apps/web/src/components/chat/ProviderCommandsPanel.tsx
Comment on lines
+60
to
+68
| const BASE_DIR = process.env.T3CODE_HOME?.trim() || Path.join(OS.homedir(), ".t3-otp"); | ||
| const STATE_DIR = Path.join(BASE_DIR, "userdata"); | ||
| const DESKTOP_SCHEME = "t3"; | ||
| const ROOT_DIR = Path.resolve(__dirname, "../../.."); | ||
| const isDevelopment = Boolean(process.env.VITE_DEV_SERVER_URL); | ||
| const APP_DISPLAY_NAME = isDevelopment ? "T3 Code (Dev)" : "T3 Code (Alpha)"; | ||
| const APP_USER_MODEL_ID = "com.t3tools.t3code"; | ||
| const USER_DATA_DIR_NAME = isDevelopment ? "t3code-dev" : "t3code"; | ||
| const LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "T3 Code (Dev)" : "T3 Code (Alpha)"; | ||
| const APP_USER_MODEL_ID = "com.t3tools.t3code-otp"; | ||
| const USER_DATA_DIR_NAME = isDevelopment ? "t3code-otp-dev" : "t3code-otp"; | ||
| const LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "T3 Code-OTP (Dev)" : "T3 Code-OTP (Alpha)"; |
There was a problem hiding this comment.
Keep the actual old Electron profile name in LEGACY_USER_DATA_DIR_NAME.
resolveUserDataPath() only checks this constant to preserve existing Chromium state. With the new "T3 Code-OTP ..." value, upgrades from existing installs stop matching the old "T3 Code ..." directory and users start with a fresh profile.
Suggested fix
const APP_USER_MODEL_ID = "com.t3tools.t3code-otp";
const USER_DATA_DIR_NAME = isDevelopment ? "t3code-otp-dev" : "t3code-otp";
-const LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "T3 Code-OTP (Dev)" : "T3 Code-OTP (Alpha)";
+const LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "T3 Code (Dev)" : "T3 Code (Alpha)";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main.ts` around lines 60 - 68, LEGACY_USER_DATA_DIR_NAME
currently uses the new "T3 Code-OTP ..." name so resolveUserDataPath() won't
detect existing Chromium profiles; change LEGACY_USER_DATA_DIR_NAME to the
original Electron profile name (use "T3 Code (Dev)" for dev and "T3 Code
(Alpha)" for prod) so the constant matches legacy directories and upgrades
preserve user state—update the LEGACY_USER_DATA_DIR_NAME definition near
BASE_DIR / resolveUserDataPath() accordingly.
Comment on lines
14
to
+15
| config :harness, | ||
| harness_secret: harness_secret, | ||
| cursor_acp_enabled: cursor_acp_enabled | ||
| harness_secret: harness_secret |
There was a problem hiding this comment.
Removing cursor_acp_enabled drops your runtime rollback lever
This removes the emergency kill-switch for Cursor routing if ACP regresses in production.
🛟 Minimal rollback-toggle restoration
harness_secret = System.get_env("T3CODE_HARNESS_SECRET", "dev-harness-secret")
+cursor_acp_enabled =
+ case String.downcase(System.get_env("T3CODE_CURSOR_ACP", "true")) do
+ value when value in ["1", "true", "yes", "on"] -> true
+ _ -> false
+ end
config :harness,
- harness_secret: harness_secret
+ harness_secret: harness_secret,
+ cursor_acp_enabled: cursor_acp_enabledBased on learnings: Keep CursorSession fallback paths until ACP replacements are proven end to end.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/harness/config/runtime.exs` around lines 14 - 15, Restore the removed
runtime toggle by adding back a harness config key named cursor_acp_enabled
(e.g., config :harness, cursor_acp_enabled: System.get_env("CURSOR_ACP_ENABLED")
|> parse_bool_or_default(true)) alongside the existing harness_secret so
CursorSession fallback/kill-switch remains available; reference the config block
where harness_secret is set and ensure CursorSession code reads :harness,
:cursor_acp_enabled to toggle ACP routing at runtime.
Comment on lines
+287
to
+289
| if (effort === null && thinkingEnabled === null) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Early-return condition hides Fast Mode-only controls.
This guard ignores caps.supportsFastMode, so the picker disappears for models that expose only Fast Mode. Include Fast Mode in the visibility condition. Also apply the same condition at Line 197 in TraitsMenuContent to keep trigger/content behavior consistent.
💡 Proposed fix
- if (effort === null && thinkingEnabled === null) {
+ if (effort === null && thinkingEnabled === null && !caps.supportsFastMode) {
return null;
}- if (effort === null && thinkingEnabled === null) {
+ if (effort === null && thinkingEnabled === null && !caps.supportsFastMode) {
return null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (effort === null && thinkingEnabled === null) { | |
| return null; | |
| } | |
| if (effort === null && thinkingEnabled === null && !caps.supportsFastMode) { | |
| return null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/chat/TraitsPicker.tsx` around lines 287 - 289, The
early-return in TraitsPicker that checks "if (effort === null && thinkingEnabled
=== null) return null" hides controls for models that only support Fast Mode;
update this guard to also consider "caps.supportsFastMode" (i.e., return only
when effort, thinkingEnabled, and caps.supportsFastMode are all null/false) so
Fast Mode controls remain visible; apply the same change to the analogous guard
in TraitsMenuContent so trigger and content visibility remain consistent (use
the same combined condition referencing effort, thinkingEnabled, and
caps.supportsFastMode).
Comment on lines
+453
to
+455
| appId: "com.t3tools.t3code-otp", | ||
| productName, | ||
| artifactName: "T3-Code-${version}-${arch}.${ext}", | ||
| artifactName: "T3-Code-OTP-${version}-${arch}.${ext}", |
There was a problem hiding this comment.
Artifact rename will break scripts/release-smoke.ts expectations unless updated
artifactName now emits T3-Code-OTP-*, but scripts/release-smoke.ts fixtures/assertions still check T3-Code-*, so smoke verification will fail after this change.
🔧 Suggested follow-up diff (cross-file alignment)
--- a/scripts/release-smoke.ts
+++ b/scripts/release-smoke.ts
@@
- - url: T3-Code-9.9.9-smoke.0-arm64.zip
+ - url: T3-Code-OTP-9.9.9-smoke.0-arm64.zip
@@
- - url: T3-Code-9.9.9-smoke.0-arm64.dmg
+ - url: T3-Code-OTP-9.9.9-smoke.0-arm64.dmg
@@
-path: T3-Code-9.9.9-smoke.0-arm64.zip
+path: T3-Code-OTP-9.9.9-smoke.0-arm64.zip
@@
- - url: T3-Code-9.9.9-smoke.0-x64.zip
+ - url: T3-Code-OTP-9.9.9-smoke.0-x64.zip
@@
- - url: T3-Code-9.9.9-smoke.0-x64.dmg
+ - url: T3-Code-OTP-9.9.9-smoke.0-x64.dmg
@@
-path: T3-Code-9.9.9-smoke.0-x64.zip
+path: T3-Code-OTP-9.9.9-smoke.0-x64.zip
@@
- "T3-Code-9.9.9-smoke.0-arm64.zip",
+ "T3-Code-OTP-9.9.9-smoke.0-arm64.zip",
@@
- "T3-Code-9.9.9-smoke.0-x64.zip",
+ "T3-Code-OTP-9.9.9-smoke.0-x64.zip",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build-desktop-artifact.ts` around lines 453 - 455, The artifactName
change to "T3-Code-OTP-${version}-${arch}.${ext}" breaks the smoke tests in
scripts/release-smoke.ts which still expect "T3-Code-*"; update the
assertions/fixtures in scripts/release-smoke.ts to match the new artifact naming
(search for checks referencing "T3-Code-" or hardcoded artifact names) and
ensure any helper/constant used for expected artifactName is updated or
centralized so both the build script (artifactName) and release-smoke.ts use the
same expected pattern.
…p, probe UX - Fix MapSet.size → map_size on tool_items (runtime crash in get_diagnostics) - Handle cursor/ask_question in reject_all_pending (emit user-input/resolved, not request/resolved) - Make cursor/create_plan fire-and-forget (acknowledge immediately, don't orphan pending entry) - Emit session/configured after set_config_option response (notify frontend of config changes) - Update explain.ex Cursor description to match ACP/JSON-RPC reality - Validate full ProviderCommand shape in deriveProviderCommands (name + description + type) - acp-probe: /mode uses set_config_option path, /quit uses rl.close() for proper cleanup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/harness/lib/harness/providers/acp_session.ex (1)
1299-1321: Command parsing helpers are well-structured with reasonable fallbacks.The
parse_command/1function and helpers correctly extract command metadata with type classification. The fallback at line 1305 usinginspect(other)is defensive, though it may produce less user-friendly names in the UI for malformed commands.Consider adding a warning log for malformed commands to aid debugging:
Optional: Add logging for malformed commands
defp parse_command(%{"name" => name}), do: %{"name" => name, "description" => "", "type" => "other"} - defp parse_command(other), do: %{"name" => inspect(other), "description" => "", "type" => "other"} + defp parse_command(other) do + Logger.warning("Received malformed command: #{inspect(other)}") + %{"name" => inspect(other), "description" => "", "type" => "other"} + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/harness/lib/harness/providers/acp_session.ex` around lines 1299 - 1321, Add a warning log when parse_command/1 receives a malformed input (the fallback clause defp parse_command(other)) so developers can spot bad command shapes; modify the fallback to call Logger.warn with a concise message that includes inspect(other) and context (e.g., "harness acp_session: malformed command") before returning the existing fallback map, keeping the other parse_command clauses unchanged and using the same function name parse_command/1 to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/harness/lib/harness/providers/acp_session.ex`:
- Around line 1299-1321: Add a warning log when parse_command/1 receives a
malformed input (the fallback clause defp parse_command(other)) so developers
can spot bad command shapes; modify the fallback to call Logger.warn with a
concise message that includes inspect(other) and context (e.g., "harness
acp_session: malformed command") before returning the existing fallback map,
keeping the other parse_command clauses unchanged and using the same function
name parse_command/1 to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5fbcdd0a-8882-45e6-8d6e-0b13b735f165
📒 Files selected for processing (4)
apps/harness/lib/harness/dev/explain.exapps/harness/lib/harness/providers/acp_session.exapps/web/src/session-logic.tsscripts/acp-probe.ts
✅ Files skipped from review due to trivial changes (1)
- apps/harness/lib/harness/dev/explain.ex
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/session-logic.ts
- scripts/acp-probe.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/src/provider/Layers/HarnessClientAdapter.ts`:
- Around line 494-504: The handler for event.method === "session/configured" is
overwriting the original payload.config and losing fields like mcp_servers;
instead merge the incoming metadata into the existing config: read the raw
payload.config (e.g., const rawConfig = payload?.config ?? {}), copy it and then
add currentModeId/currentModelId when present (e.g., mergedConfig = {
...rawConfig, ...(payload?.currentModeId && { currentModeId:
payload.currentModeId }), ...(payload?.currentModelId && { currentModelId:
payload.currentModelId }) }), and use mergedConfig as the returned
payload.config in the object created by runtimeEventBase/event.method handling
so downstream code (e.g., ProviderRuntimeIngestion) still sees mcp_servers and
other original fields.
In `@apps/web/src/components/ChatView.tsx`:
- Line 351: The commands popover state (commandsPanelOpen /
setCommandsPanelOpen) can remain true when its trigger is no longer rendered;
add a derived boolean (e.g., canShowCommandsPanel) that checks the same
conditions used to render the commands trigger/footer and, in a useEffect
watching canShowCommandsPanel, call setCommandsPanelOpen(false) whenever
canShowCommandsPanel becomes false; update all relevant places where the popover
can outlive its trigger (the current footer/compact footer/thread switches
referenced around the ChatView component) to use this guard so the panel is
closed whenever the trigger is not renderable.
- Around line 688-692: The provider commands toggle is only used in the
non-compact footer so compact mode hides the commands; compute providerCommands
via deriveProviderCommands(threadActivities) and expose the same boolean
(showCommandsButton) into the compact branch by passing it as a prop to
CompactComposerControlsMenu (or move the toggle element out of the
compact/non-compact conditional so both branches render the same entry point).
Update the call sites that render CompactComposerControlsMenu to accept a
showCommandsButton prop and use that prop to render the commands button/UI in
compact mode (referencing providerCommands, showCommandsButton,
deriveProviderCommands, CompactComposerControlsMenu, and isComposerFooterCompact
to find/modify the relevant logic).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0eb5d0b6-d8ad-41f4-aa1a-69471cc4c0d2
📒 Files selected for processing (5)
apps/server/src/provider/Layers/HarnessClientAdapter.tsapps/server/src/provider/mcpTranslation.tsapps/web/src/components/ChatView.tsxapps/web/src/components/chat/ProviderCommandsPanel.tsxscripts/acp-probe.ts
✅ Files skipped from review due to trivial changes (1)
- scripts/acp-probe.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/server/src/provider/mcpTranslation.ts
- apps/web/src/components/chat/ProviderCommandsPanel.tsx
Comment on lines
494
to
+504
| if (event.method === "session/configured") { | ||
| const config: Record<string, unknown> = {}; | ||
| if (payload?.currentModeId) config.currentModeId = payload.currentModeId; | ||
| if (payload?.currentModelId) config.currentModelId = payload.currentModelId; | ||
| return [ | ||
| { | ||
| ...runtimeEventBase(event, canonicalThreadId), | ||
| type: "session.state.changed", | ||
| payload: { state: "ready" }, | ||
| type: "session.configured" as const, | ||
| payload: { | ||
| config: Object.keys(config).length > 0 ? config : { updated: true }, | ||
| }, |
There was a problem hiding this comment.
Preserve the full session/configured payload.
apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts currently reads event.payload.config.mcp_servers from session.configured. Replacing config with { currentModeId, currentModelId } or { updated: true } drops those entries, so MCP status activities stop materializing after startup/reconfigure. If you need to surface mode/model metadata here, merge it into the raw config instead of overwriting it. Also note that session.configured is not treated as a state transition downstream, so this change no longer carries the old ready-state signal by itself.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/provider/Layers/HarnessClientAdapter.ts` around lines 494 -
504, The handler for event.method === "session/configured" is overwriting the
original payload.config and losing fields like mcp_servers; instead merge the
incoming metadata into the existing config: read the raw payload.config (e.g.,
const rawConfig = payload?.config ?? {}), copy it and then add
currentModeId/currentModelId when present (e.g., mergedConfig = { ...rawConfig,
...(payload?.currentModeId && { currentModeId: payload.currentModeId }),
...(payload?.currentModelId && { currentModelId: payload.currentModelId }) }),
and use mergedConfig as the returned payload.config in the object created by
runtimeEventBase/event.method handling so downstream code (e.g.,
ProviderRuntimeIngestion) still sees mcp_servers and other original fields.
| const [expandedWorkGroups, setExpandedWorkGroups] = useState<Record<string, boolean>>({}); | ||
| const [planSidebarOpen, setPlanSidebarOpen] = useState(false); | ||
| const [mcpPanelOpen, setMcpPanelOpen] = useState(false); | ||
| const [commandsPanelOpen, setCommandsPanelOpen] = useState(false); |
There was a problem hiding this comment.
Close the commands popover when its trigger is no longer renderable.
commandsPanelOpen lives outside the render guard, so switching into the compact footer, hiding the normal footer, or changing threads can remount the panel already open. Derive a canShowCommandsPanel guard and force this state back to false when that guard turns off.
Also applies to: 3867-3883, 3997-4055
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/ChatView.tsx` at line 351, The commands popover state
(commandsPanelOpen / setCommandsPanelOpen) can remain true when its trigger is
no longer rendered; add a derived boolean (e.g., canShowCommandsPanel) that
checks the same conditions used to render the commands trigger/footer and, in a
useEffect watching canShowCommandsPanel, call setCommandsPanelOpen(false)
whenever canShowCommandsPanel becomes false; update all relevant places where
the popover can outlive its trigger (the current footer/compact footer/thread
switches referenced around the ChatView component) to use this guard so the
panel is closed whenever the trigger is not renderable.
Comment on lines
+688
to
+692
| const providerCommands = useMemo( | ||
| () => deriveProviderCommands(threadActivities), | ||
| [threadActivities], | ||
| ); | ||
| const showCommandsButton = providerCommands.length > 0; |
There was a problem hiding this comment.
Expose provider commands in the compact footer too.
showCommandsButton only feeds the non-compact branch. When isComposerFooterCompact is true, the new commands UI disappears entirely, so narrow/mobile layouts lose the only discovery surface for provider commands. Please thread the same entry point through CompactComposerControlsMenu or move the toggle outside the compact/non-compact split.
Also applies to: 3867-3883, 3997-4055
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/ChatView.tsx` around lines 688 - 692, The provider
commands toggle is only used in the non-compact footer so compact mode hides the
commands; compute providerCommands via deriveProviderCommands(threadActivities)
and expose the same boolean (showCommandsButton) into the compact branch by
passing it as a prop to CompactComposerControlsMenu (or move the toggle element
out of the compact/non-compact conditional so both branches render the same
entry point). Update the call sites that render CompactComposerControlsMenu to
accept a showCommandsButton prop and use that prop to render the commands
button/UI in compact mode (referencing providerCommands, showCommandsButton,
deriveProviderCommands, CompactComposerControlsMenu, and isComposerFooterCompact
to find/modify the relevant logic).
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
1 participant
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
T3CODE_CURSOR_ACPremoved.cursor/ask_question,cursor/create_plantracked as pending requests (like elicitation).cursor/update_todosemitted asturn/plan/updated. No more-32601rejections.set_configthrough all 5 layers — GenServer → SessionManager → Channel (session.setConfig) → HarnessClientManager → ProviderAdapterShape. Cursor ACP supports live model/mode switching viasession/set_config_option.supportsUserInput: true(prevents session hang when Cursor asks questions),sessionModelSwitch: "in-session"(ACP supports live switch).datafield),loadSessioncapability gating,tool_itemsMap for kind lookup, dead-32800/-32801code removal,planpayload key fix (B1).scripts/acp-probe.tsinteractive CLI for ACP wire probing + 2 new real wire capture fixtures.{ name, description, type }in contracts withsession.commands.availableevent type.ProviderCommandsPanel.tsxwith fuzzy search + keyboard nav + screen reader support.Contracts changes
ProviderCommandschema:{ name: string, description: string, type: "builtin" | "user" | "project" | "other" }session.commands.availableevent type added toProviderRuntimeEventV2unionsupportsUserInput: truefor cursorsessionModelSwitch: "in-session"for cursorHarness changes (Elixir)
AcpSession: +215 LOC — command parsing,set_confighandle_call, cursor extension handlers,format_error_detail, capability gating, MCP status lifecycle reportingProviderBehaviour: optionalset_config/3callbackSessionManager:set_config/3routing withfunction_exported?guardHarnessChannel:session.setConfighandlerCursorSession: deleted (997 LOC)Server changes (Node)
HarnessClientAdapter:turn/plan/created,session/commands_available,session.configured(preservescurrentModeId) event mappings.setConfigimplementation.HarnessClientManager:setConfigWS methodProviderRuntimeIngestion:session-commandsactivity kindmcpTranslation:acpMcpServersFromResolved()for Cursor's mcpServers schemaTest plan
bun typecheckpassesmix compile --warnings-as-errorspasses🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores