feat(cursor): detect inline model errors, surface + notify (#878)#987
Open
heavygee wants to merge 10 commits into
Open
feat(cursor): detect inline model errors, surface + notify (#878)#987heavygee wants to merge 10 commits into
heavygee wants to merge 10 commits into
Conversation
When cursor-agent emits Error: T: [resource_exhausted], Error: T: [canceled], Error: T: Connection stalled, or similar inline model error strings, HAPI now classifies the failure, persists it in session metadata, and shows the operator a warning banner in the web UI rather than silently treating the turn as clean. - Add classifyCursorAgentMessage() + isCompletionClaim() classifiers with full unit test coverage (all pattern rows + benign strings) - Hook classifier into both the legacy stream-json and ACP Cursor launchers; suppress the subsequent 'ready' event so the banner persists until the operator explicitly dismisses it - Extend MetadataSchema with optional lastModelError field (additive, no migration) including priorAssistantClaimsDone heuristic - Add POST /sessions/:id/model-error/acknowledge hub endpoint that sets acknowledgedAt on the stored error - Add ModelErrorBanner component (amber, pulsing dot in session list, toggle raw error, dismiss calls acknowledge endpoint) - Expose lastModelError through SessionSummaryMetadata so the session list row can show the pulsing amber dot without fetching the full session Closes tiann#878 Co-authored-by: Cursor <cursoragent@cursor.com>
Real cursor-agent error emits come as the whole message body, not embedded in narrative prose. The original "contains" match on 'Gemini prompt failed: .*token count exceeds' false-positived on any assistant message that merely DESCRIBED the pattern (release notes, help text, an assistant summarising the feature). 2026-06-12 self-own: the new banner triggered on an assistant response that listed the patterns it watches for. Anchor both Gemini patterns to start-of-message (with leading-whitespace tolerance via trimStart). Add two regression tests: - prose containing the literal description does NOT classify - real error text with leading whitespace DOES classify The Error: T: family was already correctly anchored. Co-authored-by: Cursor <cursoragent@cursor.com>
Real cursor-agent error emits over ACP arrive with leading newlines (observed in session b52b9117: "\n\nError: T: WritableIterable is closed"). The earlier fix only added trimStart() to the Gemini family; the Error: T: family kept the raw `^` anchor and silently missed every error that came with prefix whitespace. Same bug class, partial fix. Apply trimStart() consistently across all 8 patterns. Add two regression tests: - the exact wire format from session b52b9117 - leading-whitespace variants of each Error: T: kind Net effect: a cursor-agent error that arrives with leading newlines now correctly fires the modelError banner. Previously, the catch-all unknown_t_prefix path silently dropped it and the operator saw "ready" instead of a degraded turn. Co-authored-by: Cursor <cursoragent@cursor.com>
The previous approach ran a regex classifier on every agent/message text and
fired modelError on the first match. Two rounds of bug-fixing later
(2026-06-12: trimStart, anchored Gemini patterns, leading-whitespace handling)
made it clear the abstraction was wrong: a hose carrying free-text agent
prose is not a reliable signal for typed errors. Self-own confirmed - the
classifier matched its own prose description in an assistant response.
Signals available in the existing transport that we were not using:
- AcpStdioTransport.parseStderrError already classifies stderr into
typed AcpStderrError (rate_limit / quota_exceeded / authentication /
model_not_found / unknown). The cursor launcher subscribed via
onStderrError but only emitted a generic status message and threw
away the type information.
- backend.prompt(...) rejection aggregates THREE structural signals
via AcpStdioTransport.markClosed -> rejectAllPending: transport
stream close (WritableIterable is closed), process exit during a
pending prompt, and JSON-RPC error responses. The launcher caught
rejections only to print 'Cursor Agent failed: ...' as a status.
This commit:
1. Adds mapAcpStderrToFailure(error) and classifyAcpRpcRejection(error)
to cursorAgentMessageClassifier. Both tag failures with
source='stderr' or source='rpc' so callers/tests can tell where
classification came from. classifyAcpRpcRejection returns null for
user cancellations / aborts (NOT model errors).
2. Adds CursorAgentStreamFailureSource and extends the kind union with
transport_closed, rpc_timeout, rpc_error, agent_crashed,
rate_limited, auth_failed, model_not_found, unknown_stderr,
prompt_failed. Existing text-classifier kinds preserved so prior
metadata stays valid.
3. Refactors cursorAcpRemoteLauncher: stderr handler now also routes
into the modelError pipeline; backend.prompt(...) catch block
classifies the rejection and records modelError when applicable.
Adds a single recordModelError(failure) consolidator so all three
signal paths share one emit point. Demotes the text classifier to
a fallback that no-ops if a structural signal already classified
the turn (prevents banner-flapping when the agent stringifies an
error we already caught structurally).
4. Mirrors the recordModelError consolidator in cursorLegacyRemoteLauncher
so both protocol paths share the same shape. Legacy stream-json
doesn't have ACP's structural signals so it still depends on the
text fallback - documented inline.
Tests:
- 41 classifier tests pass (was 26: +15 for structural signals,
transport_closed via WritableIterable, agent_crashed via spawn
failure, rpc_timeout, gRPC-status passthrough, user-cancellation
null return, source-tag verification).
- 135/135 cursor tests pass via cli workspace runner.
- bun typecheck clean across cli/web/hub.
Closes the rethink discussion on tiann#878. The text fallback path is
preserved (and still fires for legacy stream-json sessions), but
ACP sessions should now get classified failures from structural
signals first, with text only used as a last resort.
Co-authored-by: Cursor <cursoragent@cursor.com>
Legacy stream-json sessions do not exist in practice. Hub auto-migrates them to ACP at resume time via maybeAutoMigrateLegacyCursorSession (PR tiann#844): cursorSessionProtocol flips from 'stream-json' to 'acp' before cursorRemoteLauncher selects a launcher. The legacy launcher is reached only when migration soft-fails — a degraded fallback path, not a supported flow. Carrying duplicate model-error logic on the legacy path: - Doubles the surface for bugs in the model-error contract. - Implies legacy is a real, parity-required path (it is not). - Is dead code in practice (migration is ~100% reliable on healthy DBs). Removes recordModelError, handleTextMessageClassification, the turnHasModelError + lastAssistantText fields, the sendReady block on modelError, and the classifier import. Replaced with an inline comment documenting the rationale and pointing readers at the migration path if they ever encounter a use case. Net: -55 lines of dead code; ACP launcher remains the sole structural- signal surface for model errors. 135/135 cursor tests still pass. bun typecheck clean across cli/web/hub. Co-authored-by: Cursor <cursoragent@cursor.com>
Real session e7d9b44b (2026-06-13): cursor-agent emitted "Three of the four hit Codex's usage limit (tiann#151, tiann#153, tiann#155) - no code review delivered. Only tiann#157 actually got reviewed. Let me pull the inline comments to see Codex's specific suggestions:\n\n Error: T: [resource_exhausted] Error" as a single agent/message text event. The structural signals didn't fire (the agent didn't reject the prompt, didn't crash, didn't write to stderr - it just dumped the gRPC stringification into its own text stream and the prompt completed normally). The text classifier ran but missed it because patterns were anchored to start-of-string after trimStart() - the error sits AFTER the agent's narrative prose, not before it. Banner stayed clear, operator saw "the work is done" green dot. Switches all patterns from `^Error: T:` (start-of-string) to `^[ \t]*Error: T:` with multiline `m` flag (start-of-LINE plus optional horizontal whitespace). Three real wire formats now all classify: 1. Whole-body emit (b52b9117): "\n\nError: T: WritableIterable..." 2. Mid-stream append (e7d9b44b): "<prose>\n\nError: T: [resource..." 3. Leading-spaces (b52b9117): " Error: T: [canceled]" Bullet-listed pattern descriptions (the 2026-06-12 self-own: " - Error: T: [resource_exhausted]") still reject because the line starts with `[ \t]*` then `-`, not `Error:`. False-positive surface remains narrow: requires a line to LITERALLY begin with the marker (possibly after horizontal whitespace), which is characteristic of cursor-agent's runtime stringification, not genuine prose. Tests: - Adds regression test for the e7d9b44b wire format (real prose followed by `\n\nError: T: [resource_exhausted] Error`). - Adds regression test for bullet-list non-matching (no false positive). - Adds parametrized "all kinds work after newlines" test covering every pattern with the prose-then-newlines prefix shape. - 138/138 cursor tests pass; bun typecheck clean. Note: this fix only affects future turns. The error message in session e7d9b44b at seq 9286 stays in chat history without lastModelError set (no retroactive scan). Next failure of this shape will fire the banner. Co-authored-by: Cursor <cursoragent@cursor.com>
UI-only banner + amber pulse only fire for operators who happen to be
looking at the web UI. The whole point of model-error detection is to
catch the scenario where the agent reports "all done" with a green dot,
but actually hit a quota / transport / RPC failure mid-turn -- exactly
the moment the operator walks away.
Wire model errors into the existing NotificationHub so they ping the
operator's phone / Telegram / wrist *regardless* of whether the web UI
is in focus.
Plumbing:
- ModelErrorNotification type carrying kind, transient, rawSnippet,
priorAssistantClaimsDone, and atTs (used as dedup key).
- Optional sendModelError on NotificationChannel (mirrors
sendSessionCompletion's optional shape; channels that don't render
error variants just no-op).
- NotificationHub subscribes to session-updated; tracks last-notified
atTs per session so we fire once per distinct error event, dedupes
repeat session-updated events, skips already-acknowledged errors.
Channel implementations (this branch):
- Push (VAPID web push): skips the in-page-toast shortcut so a
backgrounded operator gets a real system-tray ping. Tag includes
atTs so distinct errors don't overwrite.
- Telegram: leading siren + bold "Model error - <kind>" so the
chat glance is visually distinct from ready/task copy.
Shared copy helper (modelErrorCopy.ts) gives every channel the same
kind-aware title and prior-claims-done warning body, so wrist / browser
toast / Telegram all read consistently.
FCM (companion app) sendModelError is added in a follow-up, since FCM
lives on a separate open upstream PR (tiann#803) and we don't want to
entangle the two branches.
Co-authored-by: Cursor <cursoragent@cursor.com>
Session 0e04ebe7 (teams structure): Cursor ACP emitted `Error: RetriableError: [canceled]` then ready. HAPI persists agent text in a codex-shaped envelope via convertAgentMessage — still the Cursor launcher path, not Codex runner — but the text classifier only matched `Error: T:` prefixes and missed it. Extend every bracket-status pattern with a parallel RetriableError variant. Regression test uses the exact wire format from seq 1992. Part of tiann#878. Co-authored-by: Cursor <cursoragent@cursor.com>
heavygee
force-pushed
the
feat/cursor-detect-inline-model-errors
branch
from
1739480 to
289515b
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Telegram model-error notifications can be dropped by Markdown parsing.
Summary
- Review mode: initial
- Found one issue in the new Telegram model-error notification path. The message enables Markdown while embedding session/raw error content, so certain ordinary names/errors can make Telegram reject the send before the user sees the alert.
Testing
- Not run (automation; static review only).
HAPI Bot
Session names and error snippets can contain Markdown metacharacters; parse_mode Markdown caused Telegram to drop the whole notification. Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
Author
|
Addressed the Telegram Markdown finding in 22801bc: dropped Siren emoji prefix retained for visual urgency without relying on bold formatting. |
There was a problem hiding this comment.
Findings
- [Major] Clear previous-turn assistant text before each prompt —
turnHasModelErroris reset at the start of a new turn, butlastAssistantTextis left from the previous turn. If turn N ends with “Done.” and turn N+1 fails before emitting any text,recordModelError()reads the stale text and setspriorAssistantClaimsDone: true, causing the banner/push copy to accuse the current turn of claiming completion when it did not. Evidencecli/src/cursor/cursorAcpRemoteLauncher.ts:238andcli/src/cursor/cursorAcpRemoteLauncher.ts:372.
Suggested fix:session.onThinkingChange(true); this.turnHasModelError = false; this.lastAssistantText = null;
Summary
- Review mode: follow-up after new commits
- Found one issue in the current diff. The prior Telegram Markdown parsing issue has been fixed in the follow-up commit.
Testing
- Not run (automation; static review only). Add a cursor ACP launcher/classification test for a successful “Done.” turn followed by a structural failure before any assistant text.
HAPI Bot
Stale assistant text from turn N made turn N+1 structural failures look like priorAssistantClaimsDone when the failing turn emitted no text. Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
Author
|
Fixed in 2f7ff3a: |
There was a problem hiding this comment.
Findings
- No issues found with confidence >= 80% in the latest diff.
Summary
- Review mode: follow-up after new commits
- The prior HAPI Bot finding about stale
lastAssistantTextin the Cursor ACP launcher is addressed in the current head. Residual risk: this was a static review; launcher behavior still depends on real cursor-agent ACP error sequencing.
Testing
- Not run (automation; static review only). Existing added tests cover classifier patterns and notification dedupe/copy paths; no manual Cursor ACP run verified here.
HAPI Bot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes #878 — detect inline cursor-agent model failures, surface them honestly, and (follow-up commits on stack) bridge transient hiccups.
Relationship to #871: #871 merged web warning styling only. This PR adds the CLI structural-first detection path (
cursorAcpRemoteLauncher), text classifier fallback (includingError: T:andError: RetriableError:prefixes),lastModelErrormetadata, hub emergency notifications (Push/Telegram), and blocks spuriousreadyon degraded turns.Not in this PR: FCM
sendModelError(depends on companion FCM #803 — soup-only bridge). Bridge prompt + opt-in auto (leg 3) lands onfeat/cursor-model-error-bridgeafter this merges and dogfoods.Test plan
bun typecheck && bun run test(cli classifier 45/45; hub notification tests)feat/cursor-detect-inline-model-errorsmanifest layer)Fixes #878