refactor(server): record Cursor replay fixtures with Effect - #13556
Conversation
| export const recordCursorAgentSdkReplayTranscript = Effect.fn( | ||
| "recordCursorAgentSdkReplayTranscript", | ||
| )(function* (input: CursorReplayRecordingInput) { | ||
| const invalid = (reason: string) => |
There was a problem hiding this comment.
invalid only forwards its arguments to new CursorReplayRecordingError, which adds an unnecessary error-construction helper. Consider constructing the tagged error at each validation or timeout boundary instead. This requires edits at multiple call sites, so there is no single-hunk suggestion.
Posted via Macroscope — Effect Service Conventions
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR substantially restructures the shared production Cursor SDK runner while also rewriting replay cancellation and buffering logic. An unresolved Medium finding reports a concrete transcript-ordering risk in the new mid-tool interruption path; the remaining comment is stylistic. Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
The Cursor recorder was an async function that called @cursor/sdk directly, with hand-rolled Promise signals and a Promise.race timeout, while the adapter reaches the SDK through CursorAgentSdkRunner. The two could drift. The live runner is now built by makeCursorAgentSdkRunner, which takes the protocol logger per opened agent. The live layer passes the native event log writer; the recorder passes a logger that appends each frame to the transcript. Recording therefore drives the exact code path the adapter uses, and agent.open/run.start/run.cancel/agent.close frames come from the runner instead of being rebuilt by hand. The recorder is an Effect.fn: Deferred for the interrupt triggers, Effect.timeoutOrElse for the 30 s waits, a Latch to hold updates between tool-call-started and run.cancel, acquireUseRelease so each agent is closed on every exit, a tagged CursorReplayRecordingError for invalid input, and Effect.sleep for the 10 ms cancel deferral. The script is an Effect CLI command run with NodeRuntime.runMain. Its scenario flag falls back to T3_CURSOR_REPLAY_SCENARIO, and the recording workspace is scoped. The docs drop the `--` before `--scenario`: pnpm 11 forwards it literally and the Effect CLI treats everything after it as operands. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The recorder held the transcript with a latch that it could only close after send returned, because updates that arrive earlier are flushed inside send and waiting there never returns. So when tool-call-started came in that early batch, the updates after it were recorded before run.cancel. The transcript is now ordered by buffering instead of waiting: after the first tool-call-started, frames are held and appended right after run.cancel is recorded. The SDK callback still pauses until the cancel is sent when send has already returned, as the async recorder did; without that pause all three live probes crashed with the SDK's AbortError. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
d874291 to
d5c9e25
Compare
29adfb3
into
t3code/codex-turn-mapping
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Maintainer question: "Why is the Cursor testkit async-based and not Effect?" The Cursor replay recorder was an
asyncfunction. It called@cursor/sdkdirectly, used hand-rolled Promise signals and aPromise.racetimeout, and rebuilt every protocol frame by hand. The adapter talks to the SDK throughCursorAgentSdkRunner, so recording and production could drift.What changed
The recorder now uses the runner.
CursorAgentSdk.tsbuilds the live runner withmakeCursorAgentSdkRunner(protocolLoggerFor). The live layer passes the native event-log writer, as before. The recorder passes a logger that appends each frame (agent.open,agent.opened,run.start,run.started,interaction.update,run.completed,run.cancel,agent.close, and resume) to the transcript. The SDK calls, the pending-update buffering beforerun.started, and the treatment of anAbortErrorfromrun.cancelas success now come from the same code the adapter runs. The runner change is a pure extraction:git diff -won that file is +27/-14.The recorder is an Effect (
CursorAdapterV2.testkit.ts, nowEffect.fn):Deferredreplaces the Promise signals: first update for the run-start interrupt, firsttool-call-startedfor the mid-tool interrupt.Effect.timeoutOrElsereplaces the 30 sPromise.race.tool-call-startedand appends them right afterrun.cancel, so the cancel always directly follows its trigger. It appends instead of waiting, because updates that arrive beforesendreturns are flushed insidesend, and waiting there would never return. Whentool-call-startedarrives aftersendreturns, the SDK callback also pauses until the cancel is sent, as the async recorder did (see the crash note below).Effect.acquireUseReleaseopens and closes each agent, soagent.closeruns (and is recorded) on every exit, including the restart before a resumed prompt.CursorReplayRecordingErrorinstead of throwing.Effect.sleep("10 millis"), with its comment unchanged. It works around the unhandledAbortErrorinside@cursor/sdkwhen cancelling synchronously from the tool-call-started callback.The script is an Effect CLI command (
record-cursor-agent-sdk-replay-fixture.ts), followingmigrate-dev-db.tsandt3-sqlite-state.ts:Command.runwithNodeServices.layerandNodeRuntime.runMain.--scenariois aFlag.Literalsover the recording names, falling back toT3_CURSOR_REPLAY_SCENARIO.--outis optional.CURSOR_API_KEYis read asConfig.Redacted.T3_CURSOR_REPLAY_MODELandT3_CURSOR_REPLAY_CWDare read throughConfig.checkpointWorkspace, so it is removed on every exit. TherunFileSystempromise bridge is gone.Docs:
docs/user/cursor.mdanddocs/orchestration-v2/testing-strategy.mddrop the--inpnpm --filter t3 record:cursor-replay -- --scenario …. The new argument parsing requires it: pnpm 11 forwards the--literally (checked: argv becomes["--","--scenario","simple"]), and the Effect CLI treats everything after--as trailing operands. With the--, the command fails withMissing required flag: --scenario. Without it, pnpm passes the flags straight through.Line delta: +545/-576 across 5 files. Most of that is reindentation from the runner extraction; ignoring whitespace it is +316/-347.
Verification
Committed Cursor transcripts are untouched (no fixture files in the diff).
vp test run src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts -t cursor: 10 passed.vp test runonOrchestratorReplayRecovery.integration.test.ts,OrchestratorReplayFixtures.contract.test.ts,CursorAgentSdk.test.ts,CursorAdapterV2.test.ts: 18 passed. Before rebasing onto the current base,CursorAdapterV2.testkit.test.tsandcursorReplayRecordingWorkspace.test.tsalso passed (31 total); test(server): remove replay harness self-tests #13531 has since removed both files.vp exec tsc --noEmit -p .inapps/server: noerror TSorwarning TS.vp run knip:check: clean at the first push. On the current base it reports one unused export,THREAD_DETAILS_PANEL_SPLIT_BUTTON_SURFACE_CLASSinapps/web, added by 99d76f2 on the base branch; this PR touches no web files.vp linton the three touched TS files: clean.Live, with
composer-2.5, recorded to a scratch dir outside the repo and not committed. The same scenarios were recorded with the old recorder (at the pre-change commit) and the new one, and compared in two ways:thinking/token/text-deltaupdates whose count depends on model output: identical for all four scenarios.agent.openoptions,run.startmessage and options,run.cancel,run.completedstatus and keys,runtime_exit,agent.close, labels), serialized byte-for-byte with ids,durationMs,result, andusagemasked: identical.simpleturn_interrupt_mid_tooltool-call-started(10 ms deferral, gate)tool-call-started,run.cancel,run.completed(cancelled),agent.closemessage_steeringthinking-delta,run.cancel,run.completed(cancelled), then run 2provider_thread_resumeagent.close:before-prompt-2,agent.resume:before-prompt-2,agent.resumed:before-prompt-2The entry-count differences are only in how many streaming deltas the model produced.
tool_call_read_onlywas also recorded with the new recorder. Its non-update entries match the committed fixture, including the prompt rewritten back to the fixture path, and nothing from the recording host's paths leaks into the transcript. As a stronger check, the four new recordings were copied over the committed fixtures andOrchestratorReplayFixtures -t cursorpassed 10/10. The fixtures were then restored.Review follow-up (updates flushed inside
send): a throwaway test with a mocked SDK whosesenddeliverstool-call-startedthentext-deltabefore returning failed on the first commit (text-deltawas recorded beforerun.cancel:1) and passes with the fix (ordertool-call-started,run.cancel:1,text-delta). The test was not committed, since the maintainer prefers no tests for the testkit. After the fix,turn_interrupt_mid_toolwas re-recorded live: the transcript endstool-call-started,run.cancel:1,run.completed:1(cancelled),agent.close.Crash note:
turn_interrupt_mid_toolrecording sometimes dies with the SDK's unhandledAbortErrorwhatever recorder runs it. It failed 1 of 3 runs with the pre-PR async recorder, 1 of 3 with this PR's first commit, and 2 of 6 with the fix. A variant that did not pause the SDK callback failed all 3 runs, which is why the pause stays. Re-run on failure; the 10 ms deferral reduces the crash rate but does not remove it.Diff grepped for
crsr_: none.Not run: repo-wide checks, and live re-recording of the other seven Cursor scenarios (they use the same single-run path as
simple).Recording note for anyone re-running this on a Linux host with pnpm:
apps/server/node_modules/@cursor/only linkssdk, not the platform package@cursor/sdk-linux-x64. The SDK looks for itscursorsandboxhelper next to the script, so any sandboxed scenario (turn_interrupt_mid_tool, the read-only ones) fails with "sandboxing is not supported in this environment", under both the old and the new recorder. I linked the platform package locally to record; nothing about that is in this PR.Model: Claude Opus 5.5 (Claude Code)
🤖 Generated with Claude Code