fix(cursor): use namespaced agent executable - #1232
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes Cursor provider executable collisions by switching all Cursor ACP launch, model discovery, provider health/update actions, and login guidance from the generic agent command to Cursor’s namespaced cursor-agent executable. It also bumps the host-daemon protocol version to ensure enrolled daemons refresh their built-in Cursor profile accordingly.
Changes:
- Update Cursor ACP built-in profile and runtime tests to launch
cursor-agentinstead ofagent. - Update host-daemon provider CLI health checks (including update command text) to target
cursor-agent. - Bump
HOST_DAEMON_PROTOCOL_VERSIONto 88 and update docs/tests describing the protocol bump and Cursor CLI install guidance.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/host-daemon-contract/test/contract.test.ts | Updates the protocol version contract test and rationale to reflect the Cursor executable change. |
| packages/host-daemon-contract/src/commands.ts | Bumps HOST_DAEMON_PROTOCOL_VERSION from 87 to 88. |
| packages/bb-app/README.md | Updates user-facing Cursor provider install guidance to reference cursor-agent. |
| packages/agent-runtime/src/provider-registry.test.ts | Updates provider registry expectations to launch Cursor via cursor-agent. |
| packages/agent-runtime/src/acp/profiles.ts | Switches the built-in Cursor ACP profile to use cursor-agent and updates related comments. |
| packages/agent-runtime/src/acp/bridge/model-catalog.ts | Updates documentation to describe Cursor model listing via cursor-agent --list-models. |
| packages/agent-runtime/src/acp/adapter.test.ts | Updates ACP adapter tests to expect cursor-agent in session and model listing commands. |
| apps/server/src/services/system/onboarding.ts | Updates onboarding login guidance for Cursor to cursor-agent login. |
| apps/host-daemon/src/provider-cli-health.ts | Updates Cursor provider CLI definition to use cursor-agent (including update command). |
| apps/host-daemon/src/provider-cli-health.test.ts | Updates host-daemon provider CLI health tests to expect cursor-agent behaviors and command lines. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
🚨 SLOP COP 🚨 · I am the SlopCop. I started this review under the I will check security, code quality, performance, architecture, and practical behavior. |
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5: BB used a generic agent command that could start the wrong tool. This change uses Cursor's specific cursor-agent command.
I found no blocking issues.
The rename covers session launch, model discovery, health checks, updates, onboarding, and documentation. The protocol increase to version 88 correctly forces old daemons to update.
Security review found no new unsafe command construction or trust-boundary change. The namespaced command reduces the risk that BB starts another provider's generic agent executable.
Performance review found no new process, network, disk, startup, or cache work. The daemon update cost is necessary for the changed local profile.
Validation results:
- All required GitHub checks pass.
- The local
agentlink points to Grok, whilecursor-agentpoints to Cursor. This result confirms the reported collision. - The installed
cursor-agentreturns a version and a valid model list. - Cursor documents
cursor-agent loginandcursor-agent update. - Cursor keeps
cursor-agentas a supported namespaced alias in its 2026 CLI change.
I found two low-severity test gaps. Direct tests do not assert cursor-agent update or cursor-agent login. Several test and story fixtures also keep the old agent name for Cursor.
I also checked for duplicate metadata and a useful refactor. The current copies serve separate daemon, runtime, and server contracts. A shared constant would cross those boundaries and would expand this small fix.
I did not run the fork code or a browser end-to-end test. This change has no browser route, and a valid end-to-end test needs an authenticated Cursor session.
I left no inline comments because I found no concrete defect on a changed line.
|
TY for fixing this! |
…thread (#1321) # Problem Every thread on the host freezes at "waiting" and only a full app restart clears it. This has now happened **four times on bb-app 0.36.0**, each time triggered by a single event the server could never store. The daemon holds **one in-memory event queue for the whole host** and reposts it as a single batch. When the head of that queue is an event the server deterministically refuses, the batch can never succeed — so every other thread's `turn/started`, `item/*` and `turn/completed` events pile up behind it and never reach the database. The UI reads the database, so every thread looks stuck. ### From the logs `~/.bb/logs/server.3.log` — the first rejection, then the same one repeating verbatim: ```json {"level":40,"time":1786360836499,"eventType":"provider/unhandled","scopeKind":"turn", "threadId":"thr_fpx3vkax5h","turnId":"auto-compact-1", "errorMessage":"Cannot append provider/unhandled for turn auto-compact-1 before turn/started is stored", "errorName":"MissingStoredTurnStartedError","msg":"Rejected daemon event before turn/started"} {"level":40,"time":1786360836616,"eventType":"provider/unhandled","scopeKind":"turn", "threadId":"thr_fpx3vkax5h","turnId":"auto-compact-1", ... } ``` Every occurrence, grouped by the turn that poisoned the queue: | Thread | Turn | Rejections | Window | |---|---|---:|---| | `thr_dwmzmanhn5` | `auto-compact-2` | 1911 | 08-07 14:57:19 → 15:27:06 (29.8 min) | | `thr_fpx3vkax5h` | `auto-compact-1` | 505 | 08-10 13:20:36 → 13:25:57 (5.3 min) | | `thr_sdc5dy277m` | `auto-compact-3` | 171 | 08-10 13:48:47 → 13:52:43 (3.9 min) | | `thr_qifimqh4a6` | `auto-compact-1` | 260 | 08-10 15:08:46 → 15:14:00 (5.2 min) | Every window ends at a restart, never at a recovery. During the 13:20 window the server logged **no thread activity whatsoever** — only the rejections: ``` 1 [plugin:connect] rpc listAccountServers failed: not_paired 5 Skipping malformed prompt history row 1 [plugin:agent-limits] disposed <- the restart ``` The 15:08 occurrence is visible directly in the database. Rows inserted per minute across all threads, spanning that window: ``` 15:03 | 90 events | 4 threads 15:04 | 91 | 1 15:05 | 66 | 1 15:06 | 22 | 1 15:07 | 32 | 1 15:08 | 35 | 2 <- poison event lands at 15:08:46 15:09 | 0 | 0 15:10 | 0 | 0 15:11 | 1 | 1 15:12 | 0 | 0 15:13 | 4 | 2 15:14 | 29 | 4 <- restart at 15:14:00 15:15 | 48 | 3 ``` Five minutes in which the whole machine persisted essentially nothing, then instant recovery on restart. Those events are gone: the queue is in-memory, so the restart that clears the wedge also discards everything held behind it, leaving a hole in each affected thread's transcript. # Root cause 1. **A provider-minted turn id is trusted.** `createUnhandledProviderEvent` falls back to reading `turnId` out of the raw provider event when the caller does not supply one: ```ts const turnId = args.turnId ?? getTurnIdFromRawEvent(args.rawEvent); ``` Codex labels its automatic-compaction traffic `auto-compact-N`. The string `auto-compact` appears nowhere in bb's source — it is entirely provider-minted, and every `provider/unhandled` event on all four affected threads carries `providerId: "codex"`. bb never opened that turn, so it never emitted a `turn/started` for it. Critically, every caller supplies `turnId` from bb's own turn registry and omits it *only when bb has no active turn* — precisely the case where a scraped id is guaranteed wrong. 2. **The server hard-rejects the orphan.** `resolveDaemonTurnStartDisposition` finds no stored `turn/started`; the escape hatch `ORPHAN_DROPPABLE_TURN_EVENT_TYPES` held only the two usage-snapshot types, so it throws `MissingStoredTurnStartedError`. 3. **The whole batch dies with it.** `/session/events` appends every event in one `immediate` transaction, so the throw rolls all of them back and returns `409 invalid_request`. 4. **The daemon reposts it forever.** The drain loop takes the entire queue as one batch and splices only on success: ```ts try { response = await options.postEvents(batch) } catch (error) { logger.error(..., "Failed to post daemon events; will retry on the next flush"); return; // queue untouched } queue.splice(0, batch.length); // only reached on success ``` The daemon already knows this class of error is permanent — `defaultRetryableForStatus(409)` is `false`, and `ServerResponseError.retryable` carries that verdict — but nothing consults it. # Fix **1. `apps/host-daemon/src/event-sink.ts` — never repost a batch the server permanently refused.** On a non-retryable `invalid_request`, the sink bisects the batch, drops the events that are undeliverable by construction, and delivers the rest. Since the server appends in one transaction and rolls back entirely on refusal, nothing was committed and re-posting the halves cannot duplicate. Isolating k bad events costs O(k log n) posts. The `invalid_request` code check is what keeps this narrow: `/session/events` also fails non-retryably with `401 unauthorized` and `401 inactive_session`, and those say nothing about the events themselves. Those must stay queued for the session the daemon is about to reopen, not be discarded one at a time — there is a regression test for exactly this. **2. `packages/agent-runtime/src/shared/provider-unhandled-event.ts` — stop trusting provider turn ids.** Only a turn id the caller vouched for scopes the event; the raw-event fallback is gone. **3. `packages/db/src/data/events.ts` — `provider/unhandled` becomes orphan-droppable.** A backstop, in the spirit of the existing comment about fork usage snapshots. An unhandled passthrough event is diagnostic only: losing one is a non-event, failing the batch it rode in with is not. Turn-content events still require a stored `turn/started`, so genuine ordering bugs are still caught. **4. The queue-backup tripwire logs at `warn`, not `debug`.** It never once fired in any of the four incidents, so there was no signal short of noticing the UI had stopped moving. Fix 1 is the load-bearing one. Fixes 2 and 3 close this particular trigger; only fix 1 stops the *next* unknown orphan event from wedging the host. ## Note on ordering of fixes 1 and 3 Fix 3 alone repairs already-enrolled daemons: an old daemon talking to a new server stops receiving 409s, so the wedge cannot recur even before it updates. Fix 1 is what makes the daemon resilient to the next unknown case. # Protocol version Bumped `HOST_DAEMON_PROTOCOL_VERSION` 99 → 100, matching the convention used by #1224, #1208, #1232, #1314 and #1236 for daemon-behaviour changes. Nothing in the wire *schema* changed, and both directions are compatible (old daemon + new server is in fact the repair path above) — the bump is here to push fix 1 out to enrolled machines rather than leave them on a build that wedges. Happy to drop it if you would rather not force an update cycle for this. # Tests Written as reproductions first, and confirmed failing against the base commit before the fix: | Test | Package | Reproduces | |---|---|---| | `ignores a provider-supplied turn id the caller did not vouch for` | `@bb/agent-runtime` | root cause — `auto-compact-1` scraped from raw params | | `drops orphan provider/unhandled events instead of failing the batch` | `@bb/db` | the batch-wide rollback | | `drops a permanently rejected event instead of retrying it forever` | `@bb/host-daemon` | the infinite repost | | `delivers events queued behind a permanently rejected event` | `@bb/host-daemon` | **the wedge itself** — healthy traffic from other threads gets through | | `accepts a batch carrying a provider/unhandled event for a turn bb never started` | `@bb/server` | end-to-end at the `/internal/session/events` route that produced the 409 | Plus guards against over-correcting: - `keeps events queued when the session, not the batch, is rejected` — a 401 must not bisect the queue away. - `keeps retrying a batch that fails for a retryable reason` — 5xx behaviour unchanged. One existing expectation changed: `codex/adapter.test.ts > translateEvent unknown codex notifications fall back to provider/unhandled` now expects thread scope. That path handles notifications which *failed* schema parsing, so nothing there vouches for the turn id; Codex notifications bb does parse still carry turn scope. Comment in the test explains it. ## Verification Rebased onto `d07c1ce28` and re-verified there. `pnpm exec turbo run test` on `@bb/db`, `@bb/agent-runtime`, `@bb/host-daemon`, `@bb/host-daemon-contract`, `@bb/server`, `@bb/integration-tests`: ``` @bb/host-daemon-contract 49 passed (49) @bb/host-daemon 526 passed (526) @bb/db 378 passed (378) @bb/server 1405 passed (1405) @bb/integration-tests 55 passed (55) @bb/agent-runtime 894 passed | 1 failed (895) ``` `typecheck` and `lint` clean across all of them. The single `@bb/agent-runtime` failure — `runtime.process-lifecycle.test.ts > bounds provider stderr while data arrives without a newline` — **fails identically on unmodified `origin/main`** and is unrelated to this change. --- Fixes #1320 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
## Summary - use Cursor's namespaced `cursor-agent` executable for ACP launch, model discovery, provider health/update actions, and login guidance - bump the host-daemon protocol to 88 so enrolled daemons refresh their built-in Cursor profile - update focused tests and the packaged-app provider documentation ## Why BB currently resolves the generic `agent` command for Cursor. When another provider owns that name earlier on `PATH`, BB launches the wrong CLI, model discovery fails, and the Cursor picker collapses to `Agent default`. Cursor's installer creates both `agent` and `cursor-agent` symlinks to the same executable. Using the namespaced alias keeps Cursor behavior intact while preventing collisions with other agent CLIs. Closes get-bb#1231. ## Validation - `pnpm exec turbo run build typecheck lint --filter=@bb/agent-runtime --filter=@bb/host-daemon --filter=@bb/server --filter=@bb/host-daemon-contract` - agent-runtime focused tests: 54 passed - host-daemon provider health tests: 16 passed - host-daemon contract tests: 35 passed - server onboarding tests: 2 passed - `git diff --check` - dev app: opened the Cursor model picker and verified Auto, Cursor Grok 4.5, GPT-5.6 Sol, Opus 5, Fable 5, Composer 2.5, and More models load from the live Cursor catalog ## Risk and rollout No UI or persisted-settings schema changes. Existing threads are untouched; new Cursor runtimes use the updated built-in profile after the daemon protocol upgrade. If `cursor-agent` is missing, the existing provider-health error and install flow remain available instead of silently launching an unrelated `agent` binary.
Summary
cursor-agentexecutable for ACP launch, model discovery, provider health/update actions, and login guidanceWhy
BB currently resolves the generic
agentcommand for Cursor. When another provider owns that name earlier onPATH, BB launches the wrong CLI, model discovery fails, and the Cursor picker collapses toAgent default.Cursor's installer creates both
agentandcursor-agentsymlinks to the same executable. Using the namespaced alias keeps Cursor behavior intact while preventing collisions with other agent CLIs.Closes #1231.
Validation
pnpm exec turbo run build typecheck lint --filter=@bb/agent-runtime --filter=@bb/host-daemon --filter=@bb/server --filter=@bb/host-daemon-contractgit diff --checkRisk and rollout
No UI or persisted-settings schema changes. Existing threads are untouched; new Cursor runtimes use the updated built-in profile after the daemon protocol upgrade. If
cursor-agentis missing, the existing provider-health error and install flow remain available instead of silently launching an unrelatedagentbinary.