feat(python-sdk): add CursorProvider to the harness - #1042
dchaudhari7177 wants to merge 3 commits into
Conversation
Adds "cursor" as a harness provider, following codex.py as the reference: same RawResult/Metrics mapping, same failure-type classification, same env and cwd handling. Notes on the parts that are not a straight copy: - The binary defaults to "agent", not "cursor". Cursor ships its headless agent under that name, so cursor_bin's default cannot follow the provider name the way codex_bin's and gemini_bin's do. A test pins it, because the mismatch is the sort of thing a later edit would quietly correct. - An unset permission_mode maps to --mode plan rather than --mode ask. Ask waits for an interactive answer and a subprocess has nobody to give one, so the run would hang to the harness timeout instead of failing. "auto" maps to --force and "plan" to --mode plan as specified. - CURSOR_API_KEY is set only when the caller supplies a key, so an inherited one is not clobbered with an empty string, and it goes through the environment rather than argv where it would be visible in ps. - --resume is omitted when the session id is empty, since the first turn of a session has none and an empty value is an error. Wired into SUPPORTED_PROVIDERS, build_provider, HarnessConfig.cursor_bin, the provider-list description and the _runner passthrough. 19 tests cover command construction, prompt-last positioning, session resume in both directions, the three permission modes, api-key handling, binary-missing, timeout, non-zero exit with and without output, kill-by-signal, malformed stream lines, and cost estimation. Part of Agent-Field#291. Closes Agent-Field#292
|
|
santoshkumarradha
left a comment
There was a problem hiding this comment.
This looks good on my side. The provider follows the existing harness contract, keeps the API key out of argv, handles session resume and failure modes, and the focused provider suite plus Ruff pass locally. I also checked the command flags against the installed Cursor CLI.
Performance
✓ No regressions detected |
santoshkumarradha
left a comment
There was a problem hiding this comment.
One integration point is still missing. Adding cursor to SUPPORTED_PROVIDERS makes harness_doctor() index PROVIDER_SPECS["cursor"], but this PR does not add that spec. The full Python CI reproduces it as test_doctor_checks_all_supported_providers_with_no_args failing with KeyError: cursor. Please add the Cursor availability/doctor spec and cover the doctor report. The focused provider tests and Ruff are otherwise clean.
Adding "cursor" to SUPPORTED_PROVIDERS without a PROVIDER_SPECS entry made harness_doctor() raise KeyError on the no-args path, which walks every supported provider. That is what turned test_doctor_checks_all_supported_ providers_with_no_args red. The spec is the one in the table whose binary does not match its provider name: Cursor ships its headless CLI as `agent`, so the doctor has to probe `agent --version`, not `cursor --version`. A comment says so, next to the same note already on CursorProvider.bin_path. Three tests: the doctor report for an installed cursor (asserting the probe command is `agent`, not `cursor`), the missing-binary report and its install command, and a standing check that every entry in SUPPORTED_PROVIDERS has either a spec or the documented claude-code wrapper branch. The last one is what catches the next provider added the same way.
|
Good catch — thank you, that was a real integration hole and the reproducer was exact.
Three tests rather than one:
Verification: The 7 On the CLA: it's showing not-signed on this PR — I'll get that sorted. |
santoshkumarradha
left a comment
There was a problem hiding this comment.
Thanks for fixing the missing doctor integration. I rechecked the updated head: the Cursor provider and doctor tests pass locally, and Ruff is clean. The provider spec now probes the correct agent binary and the coverage guard will catch future provider/spec drift. This looks good once the CLA and CI are green.
📊 Coverage gateThresholds from
✅ Gate passedNo surface regressed past the allowed threshold and the aggregate stayed above the floor. |
📐 Patch coverage gateThreshold: 80% on lines this PR touches vs
✅ Patch gate passedEvery surface whose lines were touched by this PR has patch coverage at or above the threshold. |
|
The updated implementation and all CI checks look good. The CLA is the only remaining step; once that’s signed, this should be good to go. |
AbirAbbas
left a comment
There was a problem hiding this comment.
Went through this against the real Cursor CLI rather than just reading it: installed the current release (2026.09.08-6caf4ff) and diffed every flag the provider emits against agent --help. All of them are accepted — -p, --trust, --output-format stream-json, --workspace, --mode plan, --force, --model, --resume <id>, prompt last, and CURSOR_API_KEY read from the environment. (Unknown flags and bad --mode values are rejected by the arg parser before the auth check, so a run that gets as far as "Authentication required" proves the rest parsed.) The doctor-spec commit does what it claims too: dropping _availability.py back to main makes four tests in test_harness_doctor.py fail with KeyError: 'cursor', including the generic all-providers one. ruff, the full Python suite (2386 passed) and the coverage/patch gates are green at c85837d.
One thing I think should change before this lands — the default permission_mode mapping. Details inline.
Two smaller notes, take or leave:
docs/harness-providers.mdstill has the install table without acursorrow; a user reading the docs won't find out the binary is calledagent.- The
assistantevent inRESULT_STREAMis{"type":"assistant","content":"working"}, but the shipped CLI emits{"type":"assistant","message":{"role":"assistant","content":[{"type":"text","text":...}]},"session_id":...}. Nothing breaks today because the final text and the chat id both come off theresultevent, but the fixture isn't pinning the shape the CLI actually produces.
… is unset An unset permission_mode mapped to --mode plan, which is read-only. The harness produces schema output by asking the agent to write a file, so app.harness(prompt, schema=..., provider="cursor") could never succeed, and plain task runs silently edited nothing. The premise was wrong too: --mode only accepts plan|ask, the CLI never defaults to ask, and headless runs auto-reject approval requests instead of hanging. Unset now emits neither --mode nor --force, matching codex, gemini, pi and claude-code. Also adds the cursor row to the provider install table and gives the test stream the assistant event shape the CLI emits.
|
Took both smaller notes as well, in the same commit (
Ruff (the pinned Thanks for testing this against the real CLI. The |
santoshkumarradha
left a comment
There was a problem hiding this comment.
Thanks for correcting the default permission behavior. Leaving the mode unset preserves the Cursor CLI default and allows schema runs to write their output, while the explicit plan and auto mappings remain intact. The focused provider, doctor, factory, defaults, and packaging tests pass locally, and Ruff is clean.
AbirAbbas
left a comment
There was a problem hiding this comment.
Checked this against the installed Cursor CLI (2026.09.08) rather than just the tests: every flag the provider can emit is in agent --help, bare -p is the write+shell mode as the fix assumes, the key only ever travels via CURSOR_API_KEY, and the stream-json fixture matches the CLI's bundled output formatter. Full sdk-python gates pass on the merge with main (ruff 0.15.22, run_pytest.sh 2428 passed, websockets 12/15 compat), and the harness lines the patch gate can't see are 100% covered by the cursor/doctor tests. Code is good to go.
|
Closing only because the CLA is still unsigned (status has been pending since Sep 6, reminder on Sep 8) and we keep the open queue to things that can merge. The code itself is approved by both of us and needs no further changes. Sign it at https://cla-assistant.io/Agent-Field/agentfield?pullRequest=1042 and reopen this PR (or ping me) and it goes straight into the merge queue. |
Closes #292. Part of #291.
Follows
codex.pyas the reference, per the last acceptance item: sameRawResult/Metricsmapping, same failure-type classification, sameenv/cwdhandling, sameparse_jsonl/extract_final_text/extract_token_usagepipeline.Four places I did not copy blindly
The binary is
agent, notcursor. Cursor ships its headless agent under that name, socursor_bindefaults to"agent"— the one provider whose default does not match its own name. There is a test pinning it, because that mismatch looks like a typo and is the sort of thing a later edit would helpfully "correct".An unset
permission_modemaps to--mode plan, not--mode ask. The issue's table saysNone→--mode ask, and I think that is wrong in this context:askwaits for an interactive answer, and a subprocess has nobody to give one, so the run would hang until the harness timeout rather than fail. Planning is the safe reading of "no mode stated", and it fails fast."auto"→--forceand"plan"→--mode planare as specified. Happy to change it ifaskis deliberate.CURSOR_API_KEYonly when supplied. Setting it unconditionally would clobber an inherited key with an empty string. It goes through the environment, never argv — a test asserts the key does not appear in the command, since argv is visible inps.--resumeomitted on an empty id. The first turn of a session has no chat id, and--resume ""is an error rather than a no-op.Wiring
SUPPORTED_PROVIDERS,build_provider,HarnessConfig.cursor_bin, theproviderfield's description, and the_runneroption passthrough.Tests — 19, all passing
Basic execution and exact command construction; prompt-last positioning (a flag after it would be read as part of the prompt); session resume in both directions —
--resumeout,session_idback; the three permission modes, parametrised; api-key present and absent; binary-missing →HarnessProviderUnavailable; timeout →FailureType.TIMEOUTnotCRASH; non-zero exit with output (not an error, matching codex) and without (error); kill-by-signal; malformed NDJSON lines skipped; cost estimation into metrics.test_harness_factory.py,test_harness_provider_codex.py,test_harness_defaults.pyandtest_harness_packaging.pystill pass — 56 total, nothing regressed.ruff checkclean on every file I touched.ruff format --checkclean on the new files;agentfield/types.pyreports as unformatted, but it does so on unmodifiedmaintoo, so I left it rather than mixing an unrelated reformat into this diff.Not in scope here
.agentfield_output.jsonschema validation is handled generically by the runner rather than per-provider, so there was nothing provider-specific to add — flagging that rather than silently ticking the box. If Cursor needs its own hook there, that is a separate change.