Skip to content

feat: add Cursor as a first-class HAPI agent option#3

Open
gaius-codius wants to merge 3 commits intopr/session-sort-backend-upstreamfrom
feat/cursor-agent-option
Open

feat: add Cursor as a first-class HAPI agent option#3
gaius-codius wants to merge 3 commits intopr/session-sort-backend-upstreamfrom
feat/cursor-agent-option

Conversation

@gaius-codius
Copy link
Owner

Summary

  • add cursor as a supported agent flavor across shared, hub, web, and CLI unions/schemas so it can be selected, spawned, and resumed consistently
  • implement hapi cursor with a dedicated Cursor CLI backend that runs agent in stream-json mode and maps its output into HAPI session events
  • update runner/docs/UI flavor handling so Cursor appears as a first-class option and follows codex-family behavior where appropriate

Test plan

  • bun run typecheck
  • cd web && bunx vitest run "src/components/NewSession/preferences.test.ts" "src/components/SessionList.test.tsx"
  • Manually run hapi cursor in a real environment with Cursor CLI installed
  • Manually spawn/resume a Cursor session from the web app

Expose Cursor across shared/hub/web agent selection and implement a dedicated `hapi cursor` runtime so sessions can be spawned, resumed, and displayed consistently like other supported agents.
@gaius-codius
Copy link
Owner Author

PR #3 Review: "feat: add Cursor as a first-class HAPI agent option"

Summary: This PR adds Cursor as a supported agent across all layers (shared types, CLI backend, hub routing, web UI) — 478 additions, 24 deletions across 32 files. The architecture follows established patterns from codex/gemini/opencode agent integrations. The core addition is CursorCliBackend.ts (299 lines) which spawns the Cursor CLI in stream-json mode and maps its output to HAPI session events.


Blocking

1. --force always passed to Cursor CLI, bypassing safety prompts
cli/src/cursor/CursorCliBackend.ts:118Flagged by: all 5 review agents

if (this.options.forceWrites !== false) {  // undefined !== false → true
    args.push('--force');
}

When the user doesn't pass --yolo, forceWrites is undefined (set in cursor.ts:15). Since undefined !== false is true, --force is always passed, meaning every Cursor session silently skips all safety confirmations. Fix: change to if (this.options.forceWrites === true).

2. Tool call ID correlation broken when call_id is missing
cli/src/cursor/CursorCliBackend.ts:162

const callId = toText(event.call_id) ?? randomUUID();

A new randomUUID() is generated independently for each event. If a tool_call started event and its matching completed event both lack call_id, they get different IDs, breaking UI correlation. A cache (e.g. keyed by tool name/index) should persist generated IDs across the started/completed pair.

3. No tests for CursorCliBackend — the core new module
cli/src/cursor/CursorCliBackend.ts (299 lines)

Zero test coverage for a substantial new module with 5 pure functions (toText, asRecord, buildPromptText, normalizeToolName, extractAssistantText), a complex stream parser (handleStreamLine), and process lifecycle management. For comparison, the Codex backend has 7 test files. At minimum, the pure functions and stream event dispatch logic should be unit tested.


Should-Fix

4. asRecord() doesn't exclude arrays
cli/src/cursor/CursorCliBackend.ts:36-37

Arrays pass typeof value === 'object', so asRecord([...]) returns the array cast as a Record. Called on event.tool_call and rawPayload. Fix: add && !Array.isArray(value).

5. Misleading indentation in ternary chains
hub/src/sync/syncEngine.ts:418-420 and cli/src/runner/run.ts:326-334

The cursor branch insertions break the progressive nesting pattern, making the else-branches appear to belong to the wrong condition. Cosmetic but actively misleading for future maintainers.

6. Cursor-specific logic hardcoded in generic runAgentSession.ts
cli/src/agent/runners/runAgentSession.ts:31-39,152-156

applyBackendSessionIdToMetadata hardcodes agentType !== 'cursor', and line 152 uses an unsafe as cast to duck-type getActiveSessionId — a method not on the AgentBackend interface. Should either add getActiveSessionId?() to the interface or move the logic into runCursor.ts.

7. onPermissionRequest handler stored but never invoked
cli/src/cursor/CursorCliBackend.ts:282-285

The handler is saved to a field with a void expression to suppress lint warnings, but is never called. Combined with finding #1, if --force is fixed, Cursor sessions may hang when awaiting permission. Needs at minimum a documenting comment; ideally permission support or explicit rejection.

8. --hapi-starting-mode silently discarded
cli/src/commands/cursor.ts:39-40

All other agent commands parse and forward --hapi-starting-mode. The cursor command consumes the flag but discards the value, breaking the established pattern.

9. Semicolons inconsistency in commands/cursor.ts
cli/src/commands/cursor.ts

Uses semicolons while all sibling command files (codex.ts, gemini.ts, opencode.ts, claude.ts) in the same directory omit them.

10. Gemini omitted from Local Mode docs
docs/guide/how-it-works.md:178

Lists "Claude Code, Codex, OpenCode, or Cursor" but omits Gemini, which is supported elsewhere in the same file.

11. Missing test case for 'cursor' in preferences.test.ts
web/src/components/NewSession/preferences.test.ts

The VALID_AGENTS array was updated to include 'cursor' but no test verifies loadPreferredAgent() returns 'cursor' from localStorage. One-line addition.


Nit

12. Duplicated process-killing logicCursorCliBackend.ts:261-298. cancelPrompt and disconnect are identical except the log message. Extract a shared terminateCurrentProcess() helper.

13. --started-by lacks runtime validationcommands/cursor.ts:23-24. Cast without existence check, though consistent with other commands.

14. CURSOR_PERMISSION_MODES is emptyshared/src/modes.ts:13. Means no permission UI for cursor. Internally consistent for now but should have a documenting comment.

15. CSS variable --app-flavor-cursor not definedweb/src/lib/agentFlavorUtils.ts:34. May render cursor text unstyled. Same pattern as other flavors though — likely defined at runtime.

16. Closure references session before declarationrunAgentSession.ts:31-39. Works due to JS closure semantics but is fragile.


Verdict: Request Changes

The PR is well-structured and follows existing agent integration patterns, but has 3 blocking issues that should be resolved before merging:

  1. The --force bug is a clear safety defect — all 5 review agents flagged it independently
  2. The broken tool call ID correlation will cause UI issues when call_id is absent from Cursor CLI events
  3. The core backend (299 lines) has zero test coverage despite the project having established test patterns for analogous backends

The should-fix items (especially tiann#4 asRecord array safety, tiann#6 interface violation, and tiann#7 dead permission handler) are worth addressing in this PR as well.

@gaius-codius
Copy link
Owner Author

PR #3 Review — Revised After Validation

This supersedes my previous review comment. After validating each finding against Cursor CLI documentation, codebase conventions, and actual runtime behavior, several findings were reclassified. The original review had 3 blocking issues — all have been resolved as false positives or downgraded.

Summary: This PR adds Cursor as a supported agent across all layers (shared types, CLI backend, hub routing, web UI) — 478 additions, 24 deletions across 32 files. The architecture follows established patterns from codex/gemini/opencode agent integrations. The core addition is CursorCliBackend.ts (299 lines) which spawns the Cursor CLI in stream-json mode and maps its output to HAPI session events.


No Blocking Issues

The three original blocking findings were all resolved:

  • --force always passed: Not a bug. The Cursor CLI docs confirm --force is required in print mode for file writes to be applied (without it, changes are only proposed). Since HAPI closes stdin (stdio: ['ignore', ...]), there's no interactive approval path — --force must always be passed. However, the forceWrites !== false conditional is dead code (the yolo flag has no effect). Consider either removing the forceWrites option entirely and always passing --force, or adding a comment documenting the intent.

  • Tool call ID correlation: Not a real issue. The Cursor CLI always provides call_id on every tool_call event. The randomUUID() fallback is a defensive guard, consistent with the codex backend's approach.

  • No tests for CursorCliBackend: Consistent with codebase norms. Gemini (1,246 lines) has zero backend tests. Opencode (2,443 lines) has one test for a peripheral helper. The shared AcpSdkBackend (303 lines) has zero tests. Codex's 9 test files cover utility modules, not the backend class itself.


Should-Fix

1. Misleading indentation in ternary chains
hub/src/sync/syncEngine.ts:418-420 and cli/src/runner/run.ts:326-334

Both files have genuinely misleading indentation where the cursor branch insertion breaks the progressive nesting pattern. In syncEngine.ts, the claudeSessionId fallback appears to belong to the opencode branch rather than the cursor branch. Runtime behavior is correct, but the visual structure is wrong and could lead to maintenance errors.

2. Unsafe type assertion in runAgentSession.ts
cli/src/agent/runners/runAgentSession.ts:152-153

const maybeSessionIdProvider = backend as { getActiveSessionId?: () => string | null };

getActiveSessionId is not on the AgentBackend interface. The ad-hoc cast circumvents TypeScript's type system. Should either add it as an optional method on the interface or move the logic into runCursor.ts. (The cursor-specific agentType check at line 31 is a lesser concern since runAgentSession currently only has one consumer.)

3. Semicolons inconsistency in commands/cursor.ts
cli/src/commands/cursor.ts

Uses semicolons throughout, while all sibling command files (codex.ts, gemini.ts, opencode.ts, claude.ts) in the same directory omit them.

4. Gemini omitted from Local Mode docs
docs/guide/how-it-works.md:178

Lists "Claude Code, Codex, OpenCode, or Cursor" but omits Gemini, which has full local mode support and is referenced elsewhere in the same file.


Nit

5. forceWrites conditional is dead codeCursorCliBackend.ts:118. The condition forceWrites !== false means --force is always passed regardless of the yolo flag. Since --force is required for print mode, this is correct behavior, but the option and conditional create the false impression that it can be toggled. Consider always passing --force unconditionally and removing the option, or adding a comment explaining why.

6. onPermissionRequest handler stored but never invokedCursorCliBackend.ts:282-285. Expected behavior since Cursor CLI in print mode with --force has no permission prompt flow. The void expression to suppress lint warnings is a minor code smell. A brief comment documenting that permission handling is intentionally unsupported would clarify intent.

7. --hapi-starting-mode silently discardedcommands/cursor.ts:39-40. Not a bug (cursor has no local/remote loop), but the flag is explicitly recognized just to be skipped — unlike codex which doesn't handle it at all. A comment explaining why it's discarded would help.

8. asRecord() doesn't exclude arraysCursorCliBackend.ts:36-37. Theoretical concern only — the Cursor CLI never sends arrays where records are expected, and the identical pattern exists in the codex backend. Adding && !Array.isArray(value) would be a trivial defensive improvement.

9. Duplicated process-killing logicCursorCliBackend.ts:261-298. cancelPrompt and disconnect are identical except the log message. Could extract a shared helper.

10. CURSOR_PERMISSION_MODES is emptyshared/src/modes.ts:13. Intentionally correct (Cursor CLI uses --force not a permission mode system), but a brief comment explaining why would prevent future confusion.

11. Missing 'cursor' test case in preferences.test.ts — Existing tests cover the mechanism (save/load/invalid-fallback) rather than exhaustively testing every agent string. 'opencode' is also untested. Low priority.


Verdict: Approve (with minor suggestions)

The PR is well-structured, follows established patterns, and has no blocking issues. The --force behavior is correct per Cursor CLI requirements. The should-fix items (indentation, type assertion, semicolons, docs) are all minor and could be addressed in this PR or a follow-up.

Improve maintainability and correctness by removing unsafe casts and dead options, clarifying Cursor-specific CLI behavior, and simplifying branch-specific selection logic in runner/resume paths.
Allow the agent radio list to wrap on narrow screens so all options, including Cursor, remain visible in the WebUI session creation form.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant