diff --git a/UPSTREAM_DIVERGENCE.md b/UPSTREAM_DIVERGENCE.md index af25000482b..6dfc487a6da 100644 --- a/UPSTREAM_DIVERGENCE.md +++ b/UPSTREAM_DIVERGENCE.md @@ -37,7 +37,26 @@ Sections are ordered by action: ## Ported in the current cycle -**Cycle:** 2026-04-24 · Baseline before cycle: `7c430aece` · Baseline after cycle: `ececcdcb1` +**Cycle:** 2026-04-28 · Baseline before cycle: `7f04a4a11` · Baseline after cycle: `ef574febf` + +### PR #72 — upstream sync (2026-04-28) + +| Upstream | Subject | New SHA | +| ------------------------------------------------------ | ---------------------------------------------------------- | ----------- | +| [#2364](https://github.com/pingdotgg/t3code/pull/2364) | fix(release): use configured node for smoke manifest merge | `2f1e3cc3c` | +| [#2372](https://github.com/pingdotgg/t3code/pull/2372) | Ignore stale WebSocket lifecycle events after reconnect | `b408a6857` | + +**Conflict resolutions applied:** None — both upstream commits cherry-picked cleanly. Patch context for `wsTransport.ts` / `protocol.ts` / `wsTransport.test.ts` matched MarCode HEAD exactly; only delta is the rebrand strings (`@marcode/contracts`, `"Unable to connect to the MarCode server WebSocket."`), untouched by upstream. `release-smoke.ts` line 260 matched at offset (file is shorter in MarCode because of the removed nightly-channel block, but the heredoc context is identical). + +**Note on upstream's new `isActive` socket-session gate:** The new `WsProtocolLifecycleHandlers.isActive?` callback in `protocol.ts` lives at the **socket session** layer (per-session id check inside `WsTransport.createSession`). It is distinct from the existing **per-stream** `isActive` parameter on `runStreamOnSession` in `wsTransport.ts` — they don't collide functionally but a future reader could conflate them. + +**Bundled non-upstream commit (out-of-cycle, co-shipped):** `ef574febf feat(provider): subagent task events, cursor allow_once, ACP outgoing logging` — backfill subagent test coverage for `7f04a4a11` (`task.progress` with `lastToolName` / `summary`, child-thread delta suppression), Cursor `allow_once` preference for auto-approval (preserves command-text visibility against the empty-`rawInput` Cursor server bug), and `effect-acp` `sendNotification` rewired through `logProtocol` + JSON-RPC encoding to match request/response paths. + +--- + +## Previous cycle: 2026-04-24 + +**Baseline before cycle:** `7c430aece` · **Baseline after cycle:** `ececcdcb1` ### Direct-to-main (no PR, user-approved) @@ -187,7 +206,7 @@ MarCode ships semver alphas (`1.0.0-alpha.*`), not nightly builds. Adopting nigh ## Pending real work -_None as of 2026-04-24._ Both previously-listed rows (#1996 and #2246) landed in this cycle — #1996 across PRs #69 + #70, #2246 via PR #71. Re-run the `git cherry origin/main upstream/main` workflow at the top of this doc when starting a new cycle to populate this section. +_None as of 2026-04-28._ The two upstream commits in this cycle (#2364 release-smoke node fix + #2372 stale WS lifecycle gate) landed via PR #72. Re-run the `git cherry origin/main upstream/main` workflow at the top of this doc when starting a new cycle to populate this section. --- diff --git a/apps/server/src/provider/Layers/CodexAdapter.test.ts b/apps/server/src/provider/Layers/CodexAdapter.test.ts index 5231d88b08b..37ec371c66e 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.test.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.test.ts @@ -1491,6 +1491,268 @@ lifecycleLayer("CodexAdapterLive lifecycle", (it) => { assert.equal(events[0]?.type, "item.completed"); }), ); + + // Once a subagent has been spawned, the Codex runtime forwards child-thread + // notifications through the parent's stream (rewriting `event.threadId` to + // the parent's, but preserving the original on `payload.threadId`). The + // adapter must translate those into `task.progress` events so the + // AgentGroupCard reflects live activity, and must NOT emit the normal + // `item.started` / `item.completed` mappings (which would otherwise pollute + // the parent transcript with the subagent's items). + it.effect("emits task.progress with lastToolName for child-thread item/started", () => + Effect.gen(function* () { + const { adapter, runtime } = yield* startLifecycleRuntime(); + const eventsFiber = yield* Stream.runCollect(Stream.take(adapter.streamEvents, 3)).pipe( + Effect.forkChild, + ); + + // Spawn the subagent so the tracker registers it. + yield* runtime.emit({ + id: asEventId("evt-spawn"), + kind: "notification", + provider: "codex", + createdAt: new Date().toISOString(), + method: "item/completed", + threadId: asThreadId("thread-1"), + turnId: asTurnId("turn-1"), + itemId: asItemId("collab_progress"), + payload: { + threadId: "thread-1", + turnId: "turn-1", + item: { + type: "collabAgentToolCall", + id: "collab_progress", + tool: "spawnAgent", + status: "completed", + senderThreadId: "thread-1", + receiverThreadIds: ["sub-thread-progress"], + agentsStates: { "sub-thread-progress": { status: "running" } }, + prompt: "Investigate the cache layer", + model: "gpt-5.4", + }, + }, + } satisfies ProviderEvent); + + // Child-thread item/started for a shell command. The runtime has + // rewritten event.threadId to the parent's, but payload.threadId still + // points at the subagent thread. + yield* runtime.emit({ + id: asEventId("evt-child-started"), + kind: "notification", + provider: "codex", + createdAt: new Date().toISOString(), + method: "item/started", + threadId: asThreadId("thread-1"), + turnId: asTurnId("turn-1"), + itemId: asItemId("cmd_child_1"), + payload: { + threadId: "sub-thread-progress", + turnId: "child-turn-1", + item: { + type: "commandExecution", + id: "cmd_child_1", + command: "rg --files", + commandActions: [], + cwd: "/tmp", + status: "inProgress", + }, + }, + } satisfies ProviderEvent); + + const events = Array.from(yield* Fiber.join(eventsFiber)); + // Parent's spawn produces item.completed + task.started. Child's + // item/started is suppressed and replaced by exactly one task.progress. + assert.equal(events.length, 3); + assert.equal(events[0]?.type, "item.completed"); + assert.equal(events[1]?.type, "task.started"); + const progress = events[2]; + assert.equal(progress?.type, "task.progress"); + if (progress?.type !== "task.progress") return; + assert.equal(progress.payload.taskId, "sub-thread-progress"); + assert.equal(progress.payload.description, "Investigate the cache layer"); + assert.equal(progress.payload.lastToolName, "Shell"); + // No item.started leaked into the parent transcript. + assert.equal( + events.some((e) => e.type === "item.started"), + false, + ); + }), + ); + + it.effect("emits task.progress with summary for child-thread item/completed", () => + Effect.gen(function* () { + const { adapter, runtime } = yield* startLifecycleRuntime(); + const eventsFiber = yield* Stream.runCollect(Stream.take(adapter.streamEvents, 3)).pipe( + Effect.forkChild, + ); + + yield* runtime.emit({ + id: asEventId("evt-spawn-2"), + kind: "notification", + provider: "codex", + createdAt: new Date().toISOString(), + method: "item/completed", + threadId: asThreadId("thread-1"), + turnId: asTurnId("turn-1"), + itemId: asItemId("collab_summary"), + payload: { + threadId: "thread-1", + turnId: "turn-1", + item: { + type: "collabAgentToolCall", + id: "collab_summary", + tool: "spawnAgent", + status: "completed", + senderThreadId: "thread-1", + receiverThreadIds: ["sub-thread-summary"], + agentsStates: { "sub-thread-summary": { status: "running" } }, + prompt: "Pick a random file and report it", + model: "gpt-5.4-mini", + }, + }, + } satisfies ProviderEvent); + + yield* runtime.emit({ + id: asEventId("evt-child-completed"), + kind: "notification", + provider: "codex", + createdAt: new Date().toISOString(), + method: "item/completed", + threadId: asThreadId("thread-1"), + turnId: asTurnId("turn-1"), + itemId: asItemId("msg_child_1"), + payload: { + threadId: "sub-thread-summary", + turnId: "child-turn-1", + item: { + type: "agentMessage", + id: "msg_child_1", + text: "I'll read apps/server/src/config.ts now.", + }, + }, + } satisfies ProviderEvent); + + const events = Array.from(yield* Fiber.join(eventsFiber)); + assert.equal(events.length, 3); + const progress = events[2]; + assert.equal(progress?.type, "task.progress"); + if (progress?.type !== "task.progress") return; + assert.equal(progress.payload.taskId, "sub-thread-summary"); + assert.equal(progress.payload.description, "Pick a random file and report it"); + assert.equal(progress.payload.summary, "I'll read apps/server/src/config.ts now."); + // The subagent's assistantMessage must not surface as the parent's own + // item.completed — that's what previously polluted the transcript. + const itemCompletedCount = events.filter((e) => e.type === "item.completed").length; + assert.equal(itemCompletedCount, 1); // only the parent's collabAgentToolCall + }), + ); + + it.effect("suppresses child-thread delta events to avoid polluting transcript", () => + Effect.gen(function* () { + const { adapter, runtime } = yield* startLifecycleRuntime(); + const eventsFiber = yield* Stream.runCollect(Stream.take(adapter.streamEvents, 2)).pipe( + Effect.forkChild, + ); + + yield* runtime.emit({ + id: asEventId("evt-spawn-3"), + kind: "notification", + provider: "codex", + createdAt: new Date().toISOString(), + method: "item/completed", + threadId: asThreadId("thread-1"), + turnId: asTurnId("turn-1"), + itemId: asItemId("collab_delta"), + payload: { + threadId: "thread-1", + turnId: "turn-1", + item: { + type: "collabAgentToolCall", + id: "collab_delta", + tool: "spawnAgent", + status: "completed", + senderThreadId: "thread-1", + receiverThreadIds: ["sub-thread-delta"], + agentsStates: { "sub-thread-delta": { status: "running" } }, + prompt: "Stream some text", + }, + }, + } satisfies ProviderEvent); + + // A child-thread agentMessage delta. We do NOT want this to flow through + // as a content.delta — that would render in the parent's transcript as + // if the parent assistant were speaking the subagent's words. + yield* runtime.emit({ + id: asEventId("evt-child-delta"), + kind: "notification", + provider: "codex", + createdAt: new Date().toISOString(), + method: "item/agentMessage/delta", + threadId: asThreadId("thread-1"), + turnId: asTurnId("turn-1"), + itemId: asItemId("msg_child_2"), + textDelta: "Hello from subagent", + payload: { + threadId: "sub-thread-delta", + turnId: "child-turn-1", + itemId: "msg_child_2", + delta: "Hello from subagent", + }, + } satisfies ProviderEvent); + + const events = Array.from(yield* Fiber.join(eventsFiber)); + // Only the parent's spawn item.completed + task.started flow through. + // The child's delta is silently dropped. + assert.equal(events.length, 2); + assert.equal( + events.some((e) => e.type === "content.delta"), + false, + ); + }), + ); + + // Regression guard: `payload.threadId` is the Codex provider's UUID, while + // `event.threadId` is marcode's `ThreadId` (set from `options.threadId`) — + // different namespaces that never match even for the parent's own events. + // Subagent detection MUST go via tracker membership only. A naive + // `payload.threadId !== canonicalThreadId` short-circuit would silently + // drop every parent notification, leaving the UI stuck on "Starting…". + it.effect("does not intercept parent events whose payload.threadId differs from canonical", () => + Effect.gen(function* () { + const { adapter, runtime } = yield* startLifecycleRuntime(); + const firstEventFiber = yield* Stream.runHead(adapter.streamEvents).pipe(Effect.forkChild); + + // Parent's own assistant message — payload.threadId is the provider's + // UUID, distinct from marcode's "thread-1". No spawnAgent ever fired, + // so the tracker is empty; this MUST fall through to normal mapping. + yield* runtime.emit({ + id: asEventId("evt-parent-msg"), + kind: "notification", + provider: "codex", + createdAt: new Date().toISOString(), + method: "item/completed", + threadId: asThreadId("thread-1"), + turnId: asTurnId("turn-1"), + itemId: asItemId("parent_msg"), + payload: { + threadId: "0197abcd-codex-provider-uuid", + turnId: "turn-1", + item: { + type: "agentMessage", + id: "parent_msg", + text: "I read the file.", + }, + }, + } satisfies ProviderEvent); + + const firstEvent = yield* Fiber.join(firstEventFiber); + assert.equal(firstEvent._tag, "Some"); + if (firstEvent._tag !== "Some") return; + assert.equal(firstEvent.value.type, "item.completed"); + if (firstEvent.value.type !== "item.completed") return; + assert.equal(firstEvent.value.itemId, "parent_msg"); + }), + ); }); const scopedLifecycleRuntimeFactory = makeScopedRuntimeFactory(); diff --git a/apps/server/src/provider/Layers/CodexAdapter.ts b/apps/server/src/provider/Layers/CodexAdapter.ts index 484d93f58e9..f4f8720d03d 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.ts @@ -411,6 +411,13 @@ function buildItemData(item: CodexLifecycleItem): NormalizedItemData { interface SubagentTaskState { started: boolean; terminated: boolean; + /** + * Cached subagent description (first line of the spawn prompt). Stored when + * `task.started` is emitted so subsequent child-thread events can synthesize + * `task.progress` events without re-deriving it. `task.progress`'s payload + * requires a non-empty `description`. + */ + description?: string; } export type SubagentTaskTracker = Map; @@ -508,6 +515,9 @@ function buildSubagentTaskEvents( }, }); state.started = true; + if (description) { + state.description = description; + } } } @@ -536,6 +546,197 @@ function buildSubagentTaskEvents( return events; } +/** + * Read the original `threadId` from a Codex notification payload. The runtime + * rewrites `event.threadId` to the canonical (parent) thread for child-thread + * notifications so they flow through the same stream, but the original + * provider thread id is preserved on the raw payload. We use that to detect + * subagent activity at the adapter layer. + */ +function readOriginThreadId(payload: ProviderEvent["payload"]): string | undefined { + if (payload === undefined || payload === null || typeof payload !== "object") { + return undefined; + } + const value = (payload as { readonly threadId?: unknown }).threadId; + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +/** + * Locate the `SubagentTaskState` for a child thread, given its origin + * `threadId`. Returns the canonical `taskId` (subagent thread id) plus the + * cached description so callers can synthesize `task.progress` events without + * re-walking the tracker. + */ +function findSubagentByOriginThreadId( + tracker: SubagentTaskTracker, + canonicalThreadId: ThreadId, + originThreadId: string, +): { readonly taskId: string; readonly description: string } | undefined { + const state = tracker.get(`${canonicalThreadId}|${originThreadId}`); + if (!state || !state.started || state.terminated) { + return undefined; + } + return { + taskId: originThreadId, + description: state.description ?? "Subagent task", + }; +} + +type ChildItemStarted = EffectCodexSchema.V2ItemStartedNotification["item"]; +type ChildItemCompleted = EffectCodexSchema.V2ItemCompletedNotification["item"]; + +/** + * Map a child-thread item type onto a short tool-name label for + * `task.progress.lastToolName`. The AgentGroupCard renders this as + * "Using {label}" while the subagent is mid-tool. + */ +function subagentLastToolName(item: ChildItemStarted | ChildItemCompleted): string | undefined { + if (!("type" in item)) return undefined; + switch (item.type) { + case "commandExecution": + return "Shell"; + case "fileChange": + return "Edit"; + case "webSearch": + return "WebSearch"; + case "mcpToolCall": + return `mcp__${sanitizeMcpSegment(item.server)}__${sanitizeMcpSegment(item.tool)}`; + case "dynamicToolCall": + return item.tool; + case "reasoning": + return "Thinking"; + case "agentMessage": + case "plan": + return undefined; + default: + return undefined; + } +} + +/** + * Derive a one-line activity summary from a completed child-thread item. Used + * for `task.progress.summary` so the AgentGroupCard's activity row stays in + * sync with what the subagent is actually doing. + */ +function subagentItemSummary(item: ChildItemCompleted): string | undefined { + if (!("type" in item)) return undefined; + switch (item.type) { + case "commandExecution": + return trimText(item.command); + case "fileChange": { + const firstPath = + item.changes && item.changes.length > 0 ? trimText(item.changes[0]?.path) : undefined; + return firstPath ?? "File change"; + } + case "webSearch": + return trimText(item.query); + case "mcpToolCall": + return `${sanitizeMcpSegment(item.server)}/${sanitizeMcpSegment(item.tool)}`; + case "dynamicToolCall": + return trimText(item.tool); + case "agentMessage": + return trimText(item.text); + case "reasoning": { + const summaryLine = + item.summary && item.summary.length > 0 ? trimText(item.summary[0]) : undefined; + if (summaryLine) return summaryLine; + const contentLine = + item.content && item.content.length > 0 ? trimText(item.content[0]) : undefined; + return contentLine; + } + default: + return undefined; + } +} + +/** + * Translate a child-thread notification into `task.progress` events keyed on + * the parent's subagent task id. Returns: + * - `undefined` if the event did NOT originate from a known subagent thread + * (caller should fall through to normal mapping). This includes the + * parent's own events. + * - `[]` if the event came from a known subagent but does not carry useful + * progress signal (caller should drop it so subagent items don't leak + * into the parent transcript). + * - One or more `task.progress` events otherwise. + * + * We detect child-thread events purely via `subagentTaskTracker` membership. + * Comparing `payload.threadId` to `canonicalThreadId` would be wrong — those + * are different namespaces (the marcode `ThreadId` vs Codex's provider + * thread UUID), and they never match even for the parent's own events. + * + * We only emit progress at item boundaries (`item/started` for `lastToolName`, + * `item/completed` for `summary`) — translating every text/output delta would + * make the activity row flicker and offers no incremental UX value. + */ +function buildSubagentProgressEvents( + event: ProviderEvent, + canonicalThreadId: ThreadId, + subagentTaskTracker: SubagentTaskTracker, +): ReadonlyArray | undefined { + if (event.kind !== "notification") { + return undefined; + } + const originThreadId = readOriginThreadId(event.payload); + if (!originThreadId) { + return undefined; + } + const subagent = findSubagentByOriginThreadId( + subagentTaskTracker, + canonicalThreadId, + originThreadId, + ); + if (!subagent) { + return undefined; + } + + const base = runtimeEventBase(event, canonicalThreadId); + const taskId = RuntimeTaskId.make(subagent.taskId); + const description = subagent.description; + + if (event.method === "item/started") { + const payload = readPayload(EffectCodexSchema.V2ItemStartedNotification, event.payload); + const item = payload?.item; + if (!item) return []; + const lastToolName = subagentLastToolName(item); + if (!lastToolName) return []; + return [ + { + ...base, + type: "task.progress", + payload: { + taskId, + description, + lastToolName, + }, + }, + ]; + } + + if (event.method === "item/completed") { + const payload = readPayload(EffectCodexSchema.V2ItemCompletedNotification, event.payload); + const item = payload?.item; + if (!item) return []; + const summary = subagentItemSummary(item); + const lastToolName = subagentLastToolName(item); + if (!summary && !lastToolName) return []; + return [ + { + ...base, + type: "task.progress", + payload: { + taskId, + description, + ...(summary ? { summary } : {}), + ...(lastToolName ? { lastToolName } : {}), + }, + }, + ]; + } + + return []; +} + function toRequestTypeFromMethod(method: string): CanonicalRequestType { switch (method) { case "item/commandExecution/requestApproval": @@ -873,6 +1074,16 @@ function mapToRuntimeEvents( ]; } + // Codex routes child-thread notifications through the parent's stream with + // the parent's `threadId` (see `CodexSessionRuntime.handleRawNotification`), + // so to the rest of `mapToRuntimeEvents` they look like the parent's own + // items. Translate those into `task.progress` events for the AgentGroupCard + // and suppress the normal mapping so the parent transcript stays clean. + const subagentEvents = buildSubagentProgressEvents(event, canonicalThreadId, subagentTaskTracker); + if (subagentEvents !== undefined) { + return subagentEvents; + } + if (event.method === "item/requestApproval/decision" && event.requestId) { const payload = readPayload(ApprovalDecisionPayload, event.payload); const requestType = diff --git a/apps/server/src/provider/Layers/CursorAdapter.hints.test.ts b/apps/server/src/provider/Layers/CursorAdapter.hints.test.ts index c7b3cb73c8f..701d8c3ed17 100644 --- a/apps/server/src/provider/Layers/CursorAdapter.hints.test.ts +++ b/apps/server/src/provider/Layers/CursorAdapter.hints.test.ts @@ -6,8 +6,10 @@ import { applyToolCallHint, resolveEffectiveTurnId, resolveTerminalHintFromToolCall, + selectAutoApprovedPermissionOption, } from "./CursorAdapter.ts"; import type { AcpToolCallState } from "../acp/AcpRuntimeModel.ts"; +import type * as EffectAcpSchema from "effect-acp/schema"; // Cursor ACP only emits the real command text in `session/request_permission` // (the title is backtick-wrapped, e.g. `` `rg -i 'effect'` ``). The subsequent @@ -161,3 +163,63 @@ describe("resolveEffectiveTurnId", () => { ).toBeUndefined(); }); }); + +// Full-Access mode auto-approves Cursor's `session/request_permission` events +// without prompting the user. The choice between the `allow_once` and +// `allow_always` options is load-bearing: `allow_always` makes Cursor persist +// `Shell()` to `~/.cursor/cli-config.json`, which suppresses +// future `request_permission` events for that command — and since Cursor's +// `tool_call` event always ships an empty `rawInput:{}` and the generic title +// "Terminal", losing the permission event also loses the only channel that +// carries the actual command text. The CommandExecutionCard then degrades to +// "Ran command" with no detail. Preferring `allow_once` keeps every +// invocation routed through `request_permission`, so `applyToolCallHint` +// keeps populating the command on every run. +describe("selectAutoApprovedPermissionOption", () => { + const makeRequest = ( + options: ReadonlyArray<{ readonly optionId: string; readonly kind: string }>, + ): EffectAcpSchema.RequestPermissionRequest => + ({ + sessionId: "s-1", + toolCall: { toolCallId: "t-1" }, + options: options.map((o) => ({ ...o, name: o.optionId })), + }) as EffectAcpSchema.RequestPermissionRequest; + + it("prefers `allow_once` over `allow_always` to avoid mutating cli-config", () => { + const optionId = selectAutoApprovedPermissionOption( + makeRequest([ + { optionId: "always", kind: "allow_always" }, + { optionId: "once", kind: "allow_once" }, + { optionId: "reject", kind: "reject_once" }, + ]), + ); + expect(optionId).toBe("once"); + }); + + it("falls back to `allow_always` when `allow_once` is not offered", () => { + const optionId = selectAutoApprovedPermissionOption( + makeRequest([ + { optionId: "always", kind: "allow_always" }, + { optionId: "reject", kind: "reject_once" }, + ]), + ); + expect(optionId).toBe("always"); + }); + + it("returns undefined when neither allow option is offered", () => { + const optionId = selectAutoApprovedPermissionOption( + makeRequest([{ optionId: "reject", kind: "reject_once" }]), + ); + expect(optionId).toBeUndefined(); + }); + + it("ignores allow options with empty optionId", () => { + const optionId = selectAutoApprovedPermissionOption( + makeRequest([ + { optionId: "", kind: "allow_once" }, + { optionId: "always", kind: "allow_always" }, + ]), + ); + expect(optionId).toBe("always"); + }); +}); diff --git a/apps/server/src/provider/Layers/CursorAdapter.test.ts b/apps/server/src/provider/Layers/CursorAdapter.test.ts index 553a8de4ac6..08368ca9b18 100644 --- a/apps/server/src/provider/Layers/CursorAdapter.test.ts +++ b/apps/server/src/provider/Layers/CursorAdapter.test.ts @@ -649,6 +649,12 @@ cursorAdapterTestLayer("CursorAdapterLive", (it) => { ); const requests = yield* Effect.promise(() => readJsonLines(requestLogPath)); + // Full-Access auto-approval prefers `allow-once` over `allow-always` so + // it does not mutate `~/.cursor/cli-config.json`. Persisting the + // allowlist would suppress future `session/request_permission` events, + // which is the only channel that carries the actual command text — so + // the CommandExecutionCard would degrade to "Ran command" with no + // detail. See selectAutoApprovedPermissionOption in CursorAdapter.ts. const permissionResponse = requests.find( (entry) => !("method" in entry) && @@ -660,7 +666,7 @@ cursorAdapterTestLayer("CursorAdapterLive", (it) => { "outcome" in entry.result.outcome && entry.result.outcome.outcome === "selected" && "optionId" in entry.result.outcome && - entry.result.outcome.optionId === "allow-always", + entry.result.outcome.optionId === "allow-once", ); assert.isDefined(permissionResponse); diff --git a/apps/server/src/provider/Layers/CursorAdapter.ts b/apps/server/src/provider/Layers/CursorAdapter.ts index 7dd30c8e5e7..e10421ad105 100644 --- a/apps/server/src/provider/Layers/CursorAdapter.ts +++ b/apps/server/src/provider/Layers/CursorAdapter.ts @@ -135,6 +135,18 @@ interface CursorSessionContext { // "Terminal". Stash whatever we learn from the permission request here, // keyed by `toolCallId`, so we can re-inject it into the tool_call event // and show the user the actual command that ran. + // + // KNOWN LIMITATION: when a shell command matches the user's allowlist in + // `~/.cursor/cli-config.json` (`Shell(ls)`, `Shell(rg)`, …), Cursor skips + // `request_permission` entirely AND still ships an empty `rawInput:{}` on + // the `tool_call` — so we have *no* channel to recover the command text. + // Confirmed Cursor server-side bug (Mohit, Cursor team, forum.cursor.com + // /t/acp-tool-call-events-for-mcp-tools-contain-no-tool-identity-title- + // rawinput-empty/155896): "tool_call event is emitted before tool + // arguments are fully parsed, and a deduplication guard prevents it from + // being re-emitted once the full data is available". The web-side + // CommandExecutionCard renders a "Command text unavailable" placeholder + // with a tooltip pointing users at the cli-config.json workaround. readonly toolCallHints: Map; // Terminals spawned via Cursor's `terminal/create` request, keyed by // terminalId. Cursor later references these in `session/update` tool_call @@ -388,19 +400,32 @@ export function resolveTerminalHintFromToolCall( return undefined; } -function selectAutoApprovedPermissionOption( +// Prefer `allow_once` over `allow_always` so Full-Access auto-approval does +// not mutate the user's persistent `~/.cursor/cli-config.json` allowlist. +// +// `allow_always` makes Cursor write `Shell()` (or equivalent) to +// the cli-config permissions array, which means the *next* invocation of that +// command base never triggers `session/request_permission` — and since +// Cursor's `tool_call` event ships an empty `rawInput:{}` and the generic +// title `"Terminal"`, losing the permission event also loses the only channel +// that carries the actual command text. The CommandExecutionCard then degrades +// to "Ran command" with no detail. `allow_once` keeps every invocation routed +// through `request_permission`, so `applyToolCallHint` keeps populating the +// command on every run. Falls back to `allow_always` only if the agent did +// not advertise `allow_once`. +export function selectAutoApprovedPermissionOption( request: EffectAcpSchema.RequestPermissionRequest, ): string | undefined { - const allowAlwaysOption = request.options.find((option) => option.kind === "allow_always"); - if (typeof allowAlwaysOption?.optionId === "string" && allowAlwaysOption.optionId.trim()) { - return allowAlwaysOption.optionId.trim(); - } - const allowOnceOption = request.options.find((option) => option.kind === "allow_once"); if (typeof allowOnceOption?.optionId === "string" && allowOnceOption.optionId.trim()) { return allowOnceOption.optionId.trim(); } + const allowAlwaysOption = request.options.find((option) => option.kind === "allow_always"); + if (typeof allowAlwaysOption?.optionId === "string" && allowAlwaysOption.optionId.trim()) { + return allowAlwaysOption.optionId.trim(); + } + return undefined; } diff --git a/apps/web/src/rpc/protocol.ts b/apps/web/src/rpc/protocol.ts index 8719c4125f0..4836322b982 100644 --- a/apps/web/src/rpc/protocol.ts +++ b/apps/web/src/rpc/protocol.ts @@ -18,6 +18,7 @@ import { } from "./wsConnectionState"; export interface WsProtocolLifecycleHandlers { + readonly isActive?: () => boolean; readonly onAttempt?: (socketUrl: string) => void; readonly onOpen?: () => void; readonly onError?: (message: string) => void; @@ -49,6 +50,7 @@ function resolveWsRpcSocketUrl(rawUrl: string): string { function defaultLifecycleHandlers(): Required { return { + isActive: () => true, onAttempt: recordWsConnectionAttempt, onOpen: recordWsConnectionOpened, onError: (message) => { @@ -66,21 +68,35 @@ function composeLifecycleHandlers( handlers?: WsProtocolLifecycleHandlers, ): Required { const defaults = defaultLifecycleHandlers(); + const isActive = handlers?.isActive ?? (() => true); return { + isActive, onAttempt: (socketUrl) => { + if (!isActive()) { + return; + } defaults.onAttempt(socketUrl); handlers?.onAttempt?.(socketUrl); }, onOpen: () => { + if (!isActive()) { + return; + } defaults.onOpen(); handlers?.onOpen?.(); }, onError: (message) => { + if (!isActive()) { + return; + } defaults.onError(message); handlers?.onError?.(message); }, onClose: (details) => { + if (!isActive()) { + return; + } defaults.onClose(details); handlers?.onClose?.(details); }, diff --git a/apps/web/src/rpc/wsTransport.test.ts b/apps/web/src/rpc/wsTransport.test.ts index be7babfea77..2702f404364 100644 --- a/apps/web/src/rpc/wsTransport.test.ts +++ b/apps/web/src/rpc/wsTransport.test.ts @@ -328,6 +328,11 @@ describe("WsTransport", () => { const secondSocket = getSocket(); expect(secondSocket).not.toBe(firstSocket); expect(firstSocket.readyState).toBe(MockWebSocket.CLOSED); + expect(getWsConnectionStatus()).toMatchObject({ + closeCode: null, + closeReason: null, + phase: "connecting", + }); const requestPromise = transport.request((client) => client[WS_METHODS.serverUpsertKeybinding]({ @@ -365,6 +370,58 @@ describe("WsTransport", () => { await transport.dispose(); }); + it("ignores stale socket lifecycle events after a reconnect starts a new session", async () => { + const onClose = vi.fn(); + const transport = createTransport("ws://localhost:3020", { onClose }); + + await waitFor(() => { + expect(sockets).toHaveLength(1); + }); + + const firstSocket = getSocket(); + firstSocket.open(); + + await waitFor(() => { + expect(getWsConnectionStatus()).toMatchObject({ + hasConnected: true, + phase: "connected", + }); + }); + + await transport.reconnect(); + + await waitFor(() => { + expect(sockets).toHaveLength(2); + }); + + expect(onClose).not.toHaveBeenCalled(); + expect(getWsConnectionStatus()).toMatchObject({ + closeCode: null, + closeReason: null, + phase: "connecting", + }); + + const secondSocket = getSocket(); + secondSocket.open(); + + await waitFor(() => { + expect(getWsConnectionStatus()).toMatchObject({ + phase: "connected", + }); + }); + + firstSocket.close(1006, "stale close"); + + expect(onClose).not.toHaveBeenCalled(); + expect(getWsConnectionStatus()).toMatchObject({ + closeCode: null, + closeReason: null, + phase: "connected", + }); + + await transport.dispose(); + }); + it("marks unary requests as slow until the first server ack arrives", async () => { const slowAckThresholdMs = 25; setSlowRpcAckThresholdMsForTests(slowAckThresholdMs); diff --git a/apps/web/src/rpc/wsTransport.ts b/apps/web/src/rpc/wsTransport.ts index d9a50a9fad0..0b90b22431f 100644 --- a/apps/web/src/rpc/wsTransport.ts +++ b/apps/web/src/rpc/wsTransport.ts @@ -53,6 +53,8 @@ export class WsTransport { private disposed = false; private hasReportedTransportDisconnect = false; private reconnectChain: Promise = Promise.resolve(); + private nextSessionId = 0; + private activeSessionId = 0; private session: TransportSession; constructor( @@ -215,8 +217,17 @@ export class WsTransport { } private createSession(): TransportSession { + const sessionId = this.nextSessionId + 1; + this.nextSessionId = sessionId; + this.activeSessionId = sessionId; const runtime = ManagedRuntime.make( - Layer.mergeAll(createWsRpcProtocolLayer(this.url, this.lifecycleHandlers), ClientTracingLive), + Layer.mergeAll( + createWsRpcProtocolLayer(this.url, { + ...this.lifecycleHandlers, + isActive: () => !this.disposed && this.activeSessionId === sessionId, + }), + ClientTracingLive, + ), ); const clientScope = runtime.runSync(Scope.make()); return { diff --git a/packages/effect-acp/src/protocol.test.ts b/packages/effect-acp/src/protocol.test.ts index 8aaa0810cb6..40fc5a7af9e 100644 --- a/packages/effect-acp/src/protocol.test.ts +++ b/packages/effect-acp/src/protocol.test.ts @@ -163,12 +163,43 @@ it.layer(NodeServices.layer)("effect-acp protocol", (it) => { direction: "outgoing", stage: "raw", payload: - '{"jsonrpc":"2.0","method":"session/cancel","params":{"sessionId":"session-1"},"id":"","headers":[]}\n', + '{"jsonrpc":"2.0","method":"session/cancel","params":{"sessionId":"session-1"}}\n', }, ]); }), ); + it.effect( + "emits JSON-RPC 2.0 spec-compliant notifications with no id field (regression guard)", + () => + Effect.gen(function* () { + // Regression guard: prior to this test, transport.notify went through + // upstream Effect's ndJsonRpc encoder, which emits notifications as + // {"jsonrpc":"2.0","method":"...","params":{...},"id":"","headers":[]}. + // Per JSON-RPC 2.0 §4.1, notifications MUST NOT contain an "id" member. + // Cursor's ACP agent (and other spec-compliant peers) reject the + // malformed payload with `"Method not found": session/cancel`, which + // silently breaks the in-app stop button for the Cursor provider. + const { stdio, output } = yield* makeInMemoryStdio(); + const transport = yield* AcpProtocol.makeAcpPatchedProtocol({ + stdio, + serverRequestMethods: new Set(), + }); + + yield* transport.notify("session/cancel", { sessionId: "session-1" }); + const outbound = yield* Queue.take(output); + const decoded = JSON.parse(outbound) as Record; + + assert.deepEqual(decoded, { + jsonrpc: "2.0", + method: "session/cancel", + params: { sessionId: "session-1" }, + }); + assert.notProperty(decoded, "id"); + assert.notProperty(decoded, "headers"); + }), + ); + it.effect("fails notification encoding through the declared ACP error channel", () => Effect.gen(function* () { const { stdio } = yield* makeInMemoryStdio(); diff --git a/packages/effect-acp/src/protocol.ts b/packages/effect-acp/src/protocol.ts index 204cf979c39..ef269ec2258 100644 --- a/packages/effect-acp/src/protocol.ts +++ b/packages/effect-acp/src/protocol.ts @@ -464,13 +464,34 @@ export const makeAcpPatchedProtocol = Effect.fn("makeAcpPatchedProtocol")(functi method: string, payload: unknown, ) { - yield* offerOutgoing({ - _tag: "Request", - id: "", - tag: method, - payload, - headers: [], + yield* logProtocol({ + direction: "outgoing", + stage: "decoded", + payload: { + _tag: "Request", + id: "", + tag: method, + payload, + headers: [], + }, + }); + + const encoded = yield* Effect.try({ + try: () => `${JSON.stringify({ jsonrpc: "2.0", method, params: payload })}\n`, + catch: (cause) => + new AcpError.AcpProtocolParseError({ + detail: "Failed to encode ACP message", + cause, + }), }); + + yield* logProtocol({ + direction: "outgoing", + stage: "raw", + payload: encoded, + }); + + yield* Queue.offer(outgoing, encoded).pipe(Effect.asVoid); }); const sendRequest = Effect.fn("sendRequest")(function* (method: string, payload: unknown) { diff --git a/scripts/release-smoke.ts b/scripts/release-smoke.ts index 4392fda2271..88490775a1b 100644 --- a/scripts/release-smoke.ts +++ b/scripts/release-smoke.ts @@ -257,7 +257,7 @@ try { fi found_windows_manifest=true - node ${JSON.stringify(resolve(repoRoot, "scripts/merge-update-manifests.ts"))} --platform win \ + ${JSON.stringify(process.execPath)} ${JSON.stringify(resolve(repoRoot, "scripts/merge-update-manifests.ts"))} --platform win \ "$arm64_manifest" \ "$x64_manifest" \ "$output_manifest"