Commit Graph
4 Commits
Author SHA1 Message Date
HeavyGeeandGitHub 6d2d0d4707 fix(cursor): trim #784 safety patch to marker-only on legacy stream-json path (closes #822) (#828)
* fix(cursor): drop timing heuristic from #784 intercept; scan raw payload (#801 follow-up)

PR #801 shipped a two-strategy intercept for the synthetic AskQuestion
skip response in legacy stream-json mode. Real-traffic data from a
post-merge run shows the marker-match strategy never fires (the
converter's `extractToolResult` discards the marker for tool shapes it
does not recognize, returning `{}`) and the timing-signature
defense-in-depth strategy fires only on false positives - notably the
Anthropic Vertex Claude tool calls cursor-agent surfaces in legacy
sessions, which all land as `name=unknown` with the `{}` extracted
result and frequently complete under the 500 ms threshold.

Measured on a single legacy-resumed session (`7b769423`): 1,136
`name=unknown` tool calls, 16 rewritten as `no_input_surface`, zero
actual marker strings stored anywhere in the session. The 16 rewrites
were legitimate fast tool calls (Anthropic Vertex `toolu_vrtx_*` IDs)
mischaracterized as fabricated skip responses.

Changes:
- Remove the timing-signature heuristic and its supporting state
  (started-at map, elapsed-ms calculation, latency threshold, test-only
  state reset).
- Move the marker scan from the post-`extractToolResult` output to the
  raw `tool_call` payload, so it can see the marker on stream-json
  shapes the converter does not specifically recognize. Function-shaped
  tools exclude `function.arguments` from the scan to avoid matching
  agent-controlled input. Other shapes scan the full payload (no
  agent-input field exists at the top level).
- Refresh tests: drop timing-based positive cases, add a marker-in-raw-
  payload positive case for `name=unknown` shapes, and add a regression
  that legitimate fast `name=unknown` tool calls without the marker
  pass through with `status: completed`.
- Document scope: this intercept now lives only on the legacy stream-
  json path, which only resumed pre-ACP sessions hit. New cursor remote
  sessions go through `cursorAcpBackend` and the `cursor/ask_question`
  ACP extension method (#799) - immune to this bug. The intercept
  drains with the legacy session population.

Tracking: #784. Builds on #801, complements #799.

* fix(cursor): exclude agent input from marker scan; surface top-level Anthropic tool names (Codex P2)

Codex flagged a false-positive case on the fork-stage review of this
branch (heavygee/hapi#35, P2): an Anthropic tool_use shape with a
top-level `name` (e.g. `{id, name: 'TodoWrite', input: { ... }}`) gets
labelled `name=unknown` by the converter and passes the AskQuestion
gate. If the agent's `input` quotes the synthetic-skip marker - which
happens whenever an agent edits or documents this very bug - the
intercept would rewrite a perfectly fine TodoWrite as a fabricated
skip.

Two-part fix:

1. `extractToolName` now reads the top-level `name` field as a final
   fallback. A real `TodoWrite` / `Bash` / `str_replace_based_edit_tool`
   surfaces with its actual name and is rejected by the AskQuestion
   gate before the marker scan runs. The original AskQuestion
   fabrication case still surfaces as `unknown` (per #784 issue body
   the name is stripped in the fabricated payload) and remains
   detectable.

2. Defense in depth: introduce `AGENT_INPUT_KEYS = {input, args,
   arguments}` and exclude these from the non-function shape's marker
   scan. Even if a tool reaches this code path with `name=unknown` and
   the marker buried in its `input`, the intercept won't fire on agent-
   controlled text.

Two new regression tests:

- Anthropic tool_use shape `{id, name: 'TodoWrite', input: {todos: [
  marker]}}` → passes through with `status: 'completed'`.
- `name=unknown` shape with marker only inside `input` → passes through
  with `status: 'completed'`.

All 20/20 tests pass; typecheck clean (cli + web + hub).
2026-06-08 13:30:04 +08:00
3a8693f380 feat(cursor): migrate remote sessions to ACP with model/variant pickers (#799)
* feat(cli,web,hub): migrate Cursor remote sessions to ACP with model/effort pickers

Move stream-json remote launcher to legacy path and add ACP launcher with
set_config_option model/mode sync, optimistic keepalive on config changes, and
shared catalog caching. Web gets dual base/effort Cursor pickers for session and
new-session flows; hide composer status bar when Cursor sends no usage_update.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(cli,web,shared): Cursor model picker — ACP wires + CLI sku variants

Enrich the web/mobile picker with agent --list-models SKUs grouped under
ACP wire bases, fix session-open base highlight, and keep catalog discovery
safe while the ACP transport holds the CLI lock.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(cursor-acp): apply ACP default model when web resets to Default

Web sends model: null for Default; push session/set_config_option with the
ACP default[] wire so Cursor backend matches hub state. Regression tests
for setModel(null) and applyModelConfig(null).

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(acp): clear stale agent-acp lock when owning process is gone

Check lock pid with signal 0; remove orphaned lock dirs after SIGKILL or
crash so listCursorModels can run cold probes again. Regression tests for
guard and catalog discovery.

Co-authored-by: Cursor <cursoragent@cursor.com>

* test(cursor): use live pid for ACP lock handler tests

Stale-lock cleanup clears dead pids; handler tests must simulate an
active lock with the current process pid to avoid cold probes/timeouts.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(acp): scope agent CLI lock guard to Cursor agent command only

Gemini/OpenCode/Kimi ACP sessions must not register agent-acp-active;
that blocked listCursorModels while unrelated backends were running.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(hub,web): reject Cursor model changes for local sessions

Hub returns 409 when controlledByUser is set, matching Codex. Web hides
model and variant pickers for local Cursor sessions so users do not hit
a dead RPC path. Document pre-push-review in AGENTS.md.

Verified: bun typecheck; bun run test (919 cli + 243 hub + 768 web + 46 shared).
Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(web): send stable ids for Cursor ask_question replies

Parse and submit question.id and option.id so ACP receives keys like
{ approach: ['a'] } instead of index/label. Verified: bun typecheck && bun run test.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-06-06 19:51:35 +08:00
HeavyGeeandGitHub dc0d21e05b fix(cursor): intercept fabricated Questions skipped AskQuestion result in headless mode (#784) (#801)
* fix(cursor): intercept fabricated 'Questions skipped' AskQuestion result in headless mode (#784)

When cursor-agent runs under `--print --output-format stream-json` (HAPI's
current Cursor remote launcher), the CLI returns a synthetic
`Questions skipped by the user, continue with the information you already have`
response for the `AskQuestion` tool in ~zero seconds with no error flag,
because there is no IDE surface to render the question. The underlying
model can interpret this as legitimate user consent and act on it.

This patch intercepts the synthetic result in
`cli/src/cursor/utils/cursorEventConverter.ts` and rewrites the
`tool_call`/completed event to a structured `no_input_surface` failure
(`status: 'failed'`, which downstream becomes `is_error: true`).

Detection has two strategies:

1. String match - any `tool_call`/completed payload whose serialized form
   contains the synthetic-skip marker is rewritten. This is robust to
   wherever cursor-agent stuffs the marker inside the `tool_call` object.
2. Timing + name heuristic (defense in depth) - any completion that arrives
   within 500 ms of its 'started' event with a trivial result, for a tool
   call named `AskQuestion`, `askQuestion`, `ask_question`, or the
   converter's `unknown` fallback, is also rewritten. This catches the case
   where cursor-agent changes the synthetic-string text in a future release.

The converter tracks per-call timestamps in a bounded `Map` (`<= 1024`
entries, oldest evicted on overflow) and clears entries when the
corresponding 'completed' event arrives. A small test-only reset hook
isolates state between Vitest cases.

This is a transitional safety patch. It auto-deletes when #781's ACP
launcher replaces the stream-json launcher and `cursor/ask_question`
becomes a proper bidirectional ACP method where fabrication is
structurally impossible.

Scope is intentionally tiny: only `cli/src/cursor/utils/cursorEventConverter.ts`,
its colocated Vitest file, and a section in `docs/guide/cursor.md`. No
changes to `cursorRemoteLauncher.ts`, ACP code, web normalizer, or
permission UI.

Refs: tiann/hapi#781 (long-term resolution via ACP migration)
Closes: tiann/hapi#784

* fix(cursor): gate AskQuestion intercept on tool name (#784 PR #801 review)

Address regression flagged by the HAPI auto-review bot on #801:

`containsSyntheticSkipMarker` previously stringified the entire `tool_call`
payload and matched the literal marker substring. Because this PR also adds
that exact marker to `docs/guide/cursor.md` (to document the intercept), a
Cursor `read_file` of that documentation page would surface the marker
inside `readToolCall.result.content` and be rewritten as a
`no_input_surface` failure, corrupting an unrelated, legitimate result.

The intercept is now gated on the tool name resolving to an
AskQuestion-shaped call (`AskQuestion`, `askQuestion`, `ask_question`, or
the converter's `unknown` fallback for unnamed function-shaped tools).
`read_file` / `write_file` tool calls - which have explicit `read_file`
and `write_file` names from `extractToolName` - no longer fall under the
intercept, regardless of what their payload contains.

The marker check itself now walks values recursively (string / array /
object), guarded by a `WeakSet` against cycles, instead of relying on
`JSON.stringify`. Slightly tidier; behaviour is otherwise unchanged for
the AskQuestion path.

Regression tests added:

- `read_file` result whose `content` contains the marker -> passes
  through with `status: 'completed'` and no `no_input_surface`.
- `write_file` whose serialized `args` contain the marker -> same.
- A non-AskQuestion function tool (`MyCustomTool`) whose result quotes
  the marker -> same.

All 846 cli tests pass (17 in this file). `bun run typecheck` exits 0.

* fix(cursor): scope synthetic-skip check to extracted result (#784 PR #801 review-2)

Address second Major finding from the HAPI auto-review bot on #801:

After the previous fix gated the intercept on the tool name, the marker
check still recursed into the entire `tool_call` object - which includes
`function.arguments`, the agent's own prompt text. A legitimate
AskQuestion whose prompt quotes the synthetic-skip marker (e.g. an agent
debugging this exact bug, or any prompt that pastes the marker verbatim)
would have been rewritten as `no_input_surface` even when the operator
actually answered.

Changes:

1. `extractToolResult` now extracts the cursor-side response from
   function-shaped tool calls. Previously it returned `{}` for anything
   that wasn't `readToolCall` or `writeToolCall`. It now returns
   `function.result` when present, otherwise every field of `function`
   except `name` and `arguments`. This excludes the agent's input from
   what downstream sees as the tool result, and as a side effect surfaces
   the actual cursor response for function-shaped tools (which was
   previously lost - see the #784 incident note about HAPI storing
   `output: {}` for AskQuestion in the message DB).

2. `shouldRewriteAsNoInputSurface` now searches only the extracted
   `result`, not the whole `tool_call`. The bot's exact recommendation.

3. Test added: an AskQuestion whose `arguments` quote the marker but
   whose `result` is a real user answer, with elapsed time past the
   500 ms threshold so the timing heuristic does not apply. Asserts the
   tool_result passes through with `status: 'completed'` and the
   operator's actual answer.

All 847 cli tests pass (18 in `cursorEventConverter.test.ts`).
`bun run typecheck` exits 0.

The widened `extractToolResult` scope is necessary for the marker check
to actually find the synthetic string (it lives inside `function.result`
or a sibling field), and is the bot's explicit recommendation. It also
removes the long-standing data-loss bug where AskQuestion responses were
surfaced to the message DB as opaque `{}` - regardless of fabrication.
2026-06-05 21:44:37 +08:00
Mao MrandGitHub c9be2894ac feat(cursor): add support for Cursor Agent CLI integration (#236)
* feat(cursor): add support for Cursor Agent CLI integration

- Introduced new command `hapi cursor` to start Cursor Agent sessions.
- Added functionality for resuming sessions and managing permission modes.
- Updated documentation to include Cursor Agent usage and installation instructions.
- Enhanced existing codebase to accommodate Cursor as a recognized agent flavor.
- Implemented local and remote session handling for Cursor Agent.

This update expands HAPI's capabilities by integrating support for the Cursor Agent, allowing users to leverage its features alongside existing agents.

* Remove TODO.md file as it is no longer needed following the integration of Cursor Agent CLI support. This cleanup helps streamline project documentation and reflects the completion of the associated tasks.

* feat(cursor): implement remote mode and fix --hapi-starting-mode

- Consume --hapi-starting-mode in cursor command (do not forward to agent)
- Implement cursorRemoteLauncher: spawn agent -p with stream-json, --trust
- Add cursorEventConverter for NDJSON parsing (system/assistant/tool_call/result)
- Multi-turn via --resume session_id
- Update docs: cursor supports both local and remote modes

Made-with: Cursor

* fix: type error

* fix(cursor): address PR review - model UI, sessionId metadata, duplicate flags

- HappyComposer: use isClaudeFlavor for model mode (cursor has no model modes)
- cursorLocalLauncher: call onSessionFound for resume so cursorSessionId in metadata
- cursorCommand: do not forward parsed flags to cursorArgs (avoid duplicates)

Made-with: Cursor
2026-03-03 10:02:47 +08:00