feat(cursor): detect inline model errors, surface, notify, and bridge (#878)

[heavygee]

Summary

Completes tiann/hapi#878 in one PR: detect inline cursor-agent model failures, surface them honestly, emergency-notify, and bridge & retry transient hiccups.

Relationship to #871: #871 merged web warning styling only. This PR adds the CLI structural-first detection path (cursorAcpRemoteLauncher), text classifier fallback (including Error: T: and Error: RetriableError: prefixes), lastModelError metadata, hub emergency notifications (Push/Telegram/FCM), blocks spurious ready on degraded turns, and adds Bridge & retry.

Bridge & retry (leg 3, folded in)

• Manual Bridge & retry from the model-error banner (re-sends last user message with bridge context).
• Identity / dedupe via bridgedForEventId (not wall-clock atTs).
• Opt-in auto-bridge (Settings → Chat): default off. When enabled, Cursor sessions re-send the last user message once after a recoverable / transient model error.
• Hub RPC + REST routes for bridge when the CLI is available.

Explicitly out of scope

• Does not fix tiann/hapi#1472 (residual ACP exit 143 / false transport_closed). Auto-bridge may interact poorly with that residual on multi-session estates — leave auto-bridge off until bug(cursor): residual ACP exit 143 / false transport_closed after #835 (live PID, sole session) #1472 is addressed, or use manual bridge only.

Not in this PR: separate FCM soup layer is no longer required for model-error wrist push (FcmNotificationChannel.sendModelError is included here; depends on upstream FCM channel from #803).

Test plan

• bun typecheck
• Bridge unit tests (cursorModelErrorBridge.test.ts, ModelErrorBanner.test.ts)
• Hub + web test suites green on thinned tip (peer evidence)
• CI green on this tip after fold push
• Dogfood detect path on operator soup
• Dogfood Bridge & retry with auto-bridge off
• Confirm auto-bridge default remains off in settings

Fixes #878

[github-actions]

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

[heavygee]

Addressed the Telegram Markdown finding in 22801bc: dropped parse_mode: 'Markdown' and send model-error notifications as plain text. Session names and raw snippets can contain _, `, *, etc.; Telegram rejects the whole message on parse failure, which silently dropped the notification.

Siren emoji prefix retained for visual urgency without relying on bold formatting.

[github-actions]

Findings

• [Major] Clear previous-turn assistant text before each prompt — turnHasModelError is reset at the start of a new turn, but lastAssistantText is 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 sets priorAssistantClaimsDone: true, causing the banner/push copy to accuse the current turn of claiming completion when it did not. Evidence cli/src/cursor/cursorAcpRemoteLauncher.ts:238 and cli/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

[heavygee]

Fixed in 2f7ff3a: lastAssistantText is cleared alongside turnHasModelError at the start of each prompt so priorAssistantClaimsDone only reflects text from the current turn.

[github-actions]

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 lastAssistantText in 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

[github-actions]

Findings

• [Major] Bind model-error acknowledgment to the displayed error — the dismiss request currently sends only the session id, and the hub writes acknowledgedAt onto whatever metadata.lastModelError is current when the request is handled. If a second model error arrives after the page rendered but before the user taps Dismiss, this endpoint can acknowledge the newer, unseen error and remove the banner/dot for the failure that should still demand attention. Evidence: web/src/api/client.ts:424 sends no error identity, and hub/src/web/routes/sessions.ts:339 acknowledges by session id only.
  Suggested fix:

  // web: send the error instance the user dismissed
  await props.api.acknowledgeModelError(props.session.id, err.atTs)
  
  // hub/cache: only acknowledge the same error; force refresh on races
  if (currentMetadata.lastModelError.atTs !== acknowledgedAtTs) {
      throw new Error('Model error changed; refresh before acknowledging.')
  }

Summary
Review mode: follow-up after new commits
One major issue found in the latest full diff. The follow-up commit only changed the RetriableError [resource_exhausted] transient classification, but the full PR still has an acknowledgment race in the new model-error UI/API flow.

Testing
Not run (automation; static review only).

HAPI Bot

[github-actions]

Findings

• [Major] Bind model-error acknowledgment to the displayed error — the dismiss flow still sends only the session id, and the hub writes acknowledgedAt onto whichever metadata.lastModelError is current when the request is handled. If a newer model error arrives after the page rendered but before the user taps Dismiss, this can acknowledge the newer unseen error and hide its banner/dot. Evidence: web/src/api/client.ts:433 sends no error identity, and hub/src/web/routes/sessions.ts:366 acknowledges by session id only.
  Suggested fix:

  // web: send the error instance the user dismissed
  const atTs = props.session.metadata?.lastModelError?.atTs
  if (typeof atTs === 'number') {
      await props.api.acknowledgeModelError(props.session.id, atTs)
  }
  
  // hub/cache: only acknowledge that same error instance
  if (!currentMetadata.lastModelError || currentMetadata.lastModelError.atTs !== acknowledgedAtTs) {
      throw new Error('Model error changed; refresh before acknowledging.')
  }

Summary

• Review mode: follow-up after new commits
• One Major issue remains in the latest full diff. The prior bot acknowledgment-race finding is still present: the new endpoint/client path does not bind dismissal to lastModelError.atTs.

Testing

• Not run (automation; static review only).

HAPI Bot

[heavygee]

Addressed the acknowledge-binding Major in a3bc9a8:

• POST /sessions/:id/model-error/acknowledge now requires { atTs } (displayed error)
• Hub refuses with 409 if metadata.lastModelError.atTs no longer matches
• Web client + SessionChat dismiss pass the banner's atTs

Prior Majors still present on tip:

• Telegram plain text (no parse_mode) — f371fd1 / still in hub/src/telegram/bot.ts
• lastAssistantText = null at prompt start — cec46d1 / still in cursorAcpRemoteLauncher.ts

Tests: POST /sessions/:id/model-error/acknowledge (forwards atTs / 400 missing / 409 mismatch).

[github-actions]

Findings

• [Minor] Render modelError events before persisting them into chat UI — recordModelError now sends a persisted modelError agent event, but the web event presenter has no case for that type, so the chat thread falls through to JSON.stringify(event) and shows a raw JSON system row containing rawSnippet in addition to the dedicated banner. Evidence: cli/src/cursor/cursorAcpRemoteLauncher.ts:431; related fallback web/src/chat/presentation.ts:212.
  Suggested fix:

  // web/src/chat/presentation.ts
  if (event.type === 'modelError') {
      const kind = typeof event.kind === 'string' ? event.kind : 'unknown'
      return { icon: null, text: `Model error: ${kind}` }
  }

Questions

• None.

Summary

• Review mode: follow-up after new commits
• One Minor UI regression found in the latest full diff. The previous bot finding about acknowledging the displayed error is addressed by passing atTs through the client, route, engine, and cache.

Testing

• Not run: bun is not installed in this runner (bun: command not found).

HAPI Bot

[heavygee]

Cleared the Minor in da14312: getEventPresentation now handles modelError as a short kind label (banner remains the primary UI), so chat no longer JSON-dumps rawSnippet.

[github-actions]

Findings

• No issues found.

Questions

• None.

Summary

• Review mode: follow-up after new commits
• Full diff reviewed. The previous bot finding is addressed by the new modelError presentation case and regression test. Residual risk: local test execution was unavailable in this runner.

Testing

• Not run: bun is not installed in this runner (bun: command not found).

HAPI Bot

[heavygee]

Rebase onto upstream/main (0.23.4)

Rebased feat/cursor-detect-inline-model-errors onto upstream/main @ 8eac26726 (Release 0.23.4).

• New tip: 294b3ea84
• Previous tip: 0f5b978cd
• Conflict: one import-region in cli/src/cursor/cursorAcpRemoteLauncher.ts — kept both registerAcpSessionTitleSync (fix(acp): sync native agent session titles #1028) and classifier imports; classifier + banner intact.
• Guards: no skill_lookup user-turn prepend reintroduced (prompt remains bare batch.message; skillLookup stays on MCP bridge only). existingSessionId path in runCursor.ts untouched.
• Verify: bun typecheck clean; classifier (46) + ACP launcher (19 via vitest) + sessions ack routes (58) + presentation (17) green.

Force-pushed with lease to heavygee:feat/cursor-detect-inline-model-errors. Not merging (maintainer only).

[github-actions]

Findings

• [Major] RPC [canceled] Operation aborted errors are filtered out before gRPC classification — classifyAcpRpcRejection returns null for any message containing aborted, so a JSON-RPC rejection like Error: T: [canceled] Operation aborted never reaches the existing classifyCursorAgentMessage(raw) pass. The text path has regression coverage for that exact [canceled] Operation aborted shape, and this PR explicitly relies on RPC-side gRPC strings being classified, so the structural-first path can still miss degraded turns and emit a spurious ready state. Evidence: cli/src/cursor/cursorAgentMessageClassifier.ts:220
  Suggested fix:

  const textMatch = classifyCursorAgentMessage(raw)
  if (textMatch) {
      return { ...textMatch, source: 'rpc' }
  }
  
  if (lower.includes('aborted by user') || lower.includes('user cancelled') || lower.includes('user canceled')) {
      return null
  }

Questions

• None.

Summary

• Review mode: follow-up after new commits
• Full diff reviewed. One blocking behavior remains in the new RPC classifier: a real canceled model-side gRPC error can be suppressed by the broad user-abort guard.

Testing

• Not run (automation): bun is not installed in this runner (bun: command not found).

HAPI Bot

[heavygee]

Follow-up: RPC [canceled] Operation aborted Major

Addressed bot finding on tip 294b3ea84:

• Fix: c6b57a7f1 — classifyAcpRpcRejection runs classifyCursorAgentMessage(raw) before the user-abort filter; abort filter narrowed to aborted by user / user cancelled / user canceled.
• Test: regression for Error: T: [canceled] Operation aborted → kind: canceled, source: rpc.
• Resolved the review thread.

New tip: c6b57a7f1

[github-actions]

Findings

• [Major] Do not send raw model errors through external notification channels — rawSnippet is captured from provider/RPC stderr and persisted as the first 400 chars in cli/src/cursor/cursorAcpRemoteLauncher.ts:411, then formatModelErrorBody appends it into the shared notification body at hub/src/notifications/modelErrorCopy.ts:58. Both Web Push and Telegram use that body (hub/src/push/pushNotificationChannel.ts:173, hub/src/telegram/bot.ts:283), so a provider error that includes prompt text, file paths, auth details, or headers gets copied out of the local hub into third-party notification services. Keep the raw excerpt in the authenticated web banner only, or redact it before external delivery.
  Suggested fix:

  export function formatModelErrorBody(
      notification: ModelErrorNotification,
      context: { agentName: string; sessionName: string }
  ): string {
      const lines: string[] = []
      if (notification.priorAssistantClaimsDone) {
          lines.push('Agent claimed completion before this error - work likely INCOMPLETE.')
      }
      lines.push(`${context.agentName} - ${context.sessionName}`)
      // Raw provider/RPC text stays in the authenticated web banner only.
      if (notification.transient) {
          lines.push('(transient - safe to retry)')
      }
      return lines.join('\n')
  }

Questions

• None.

Summary

• Review mode: follow-up after new commits
• One Major privacy/security issue found in the notification copy path. The follow-up commit around [canceled] Operation aborted ordering does not change this finding.

Testing

• Not run (automation). Suggested coverage: update hub/src/notifications/modelErrorCopy.test.ts to assert external notification bodies omit raw snippets, while the web banner can still reveal rawSnippet inside the authenticated app.

HAPI Bot

[heavygee]

Follow-up: omit rawSnippet from external notifications

Addressed bot Major on tip c6b57a7f1:

• Fix: <new tip> — formatModelErrorBody no longer appends rawSnippet. Web Push / Telegram / FCM get kind title + agent/session + optional transient / prior-done lines only. Authenticated ModelErrorBanner still shows the raw excerpt.
• Tests: modelErrorCopy.test.ts asserts secretish raw text never appears in the body.
• Resolved the review thread.

New tip: will be filled after push.

[heavygee]

Follow-up: omit rawSnippet from external notifications

Addressed bot Major on tip c6b57a7f1:

• Fix: a43bcf5f4 — formatModelErrorBody no longer appends rawSnippet. Web Push / Telegram / FCM get kind title + agent/session + optional transient / prior-done lines only. Authenticated ModelErrorBanner still shows the raw excerpt.
• Tests: modelErrorCopy.test.ts asserts secretish raw text never appears in the body.
• Resolved the review thread.

New tip: a43bcf5f4

[github-actions]

Findings

• [Minor] Preserve same-message completion claims before text-classified errors — when Cursor appends the error to the same assistant text block, the new warning is lost; see inline comment.

Questions

• None.

Summary

• Review mode: follow-up after new commits
• One Minor issue found in the current head. The prior raw-snippet notification leakage finding is not present in this revision.

Testing

• Not run (automation). Suggested coverage: add a launcher-level test where a text message starts with a completion phrase and ends with Error: T: [resource_exhausted], asserting priorAssistantClaimsDone: true.

HAPI Bot

[heavygee]

Follow-up: same-message priorAssistantClaimsDone Minor

• Fix: 7db291c64 — when failure.source === 'text', also run isCompletionClaim(failure.raw) so Done.\n\nError: T: … sets the incomplete-work warning.
• Regression test on the combined Done+Error block.
• Resolved the review thread.

New tip: 7db291c64

[github-actions]

Findings

• [Major] Preserve lastModelError across sparse metadata writes — this field is now durable alert state, but updateSessionMetadata() still only carries forward the pre-existing allowlist (path, host, routing fields, resume tokens). The store comments document that sparse/stale metadata writes already happen; after this addition, any such write that omits lastModelError can erase an unacknowledged model-error banner or drop acknowledgedAt, so the user may lose the alert state without dismissing it. Evidence: shared/src/schemas.ts:81, related merge context hub/src/store/sessions.ts:121.
  Suggested fix:

  const ALERT_STATE_FIELDS = ['lastModelError'] as const
  
  export function mergeSessionMetadata(prior: unknown, next: unknown): unknown {
      if (!isPlainObject(prior) || !isPlainObject(next)) {
          return next
      }
      let merged: Record<string, unknown> | null = null
      merged = carryForwardIfMissing(prior, next, merged, PARSE_IDENTITY_FIELDS)
      merged = carryForwardIfMissing(prior, next, merged, ROUTING_FIELDS)
      merged = carryForwardIfMissing(prior, next, merged, SIMPLE_RESUME_TOKENS)
      merged = carryForwardIfMissing(prior, next, merged, ALERT_STATE_FIELDS)
      merged = preserveCursorProtocolPair(prior, next, merged)
      return merged ?? next
  }

Questions

• None.

Summary

• Review mode: follow-up after new commits
• One Major issue found: the new model-error metadata can be lost by existing sparse metadata replacement paths unless the store merge preserves it. Prior same-message completion review feedback is addressed in this head.

Testing

• Not run (automation). Suggested coverage: add a store-level mergeSessionMetadata/updateSessionMetadata test where prior metadata has lastModelError with acknowledgedAt, next metadata omits it, and the field is preserved.

HAPI Bot

[heavygee]

Follow-up: preserve lastModelError on sparse metadata writes

Bot Major on tip 7db291c64 — fixed in f8b9b2c10:

• mergeSessionMetadata now carry-forwards ALERT_STATE_FIELDS = ['lastModelError'] (same pattern as path/host/resume tokens).
• Test: sparse archive no longer drops an unacknowledged lastModelError.
• Thread resolved.

New tip: f8b9b2c10

[github-actions]

Findings

• No issues found with confidence >= 80% in the latest full diff.

Questions

• None.

Summary

• Review mode: follow-up after new commits
• Full diff reviewed. The previous HAPI Bot finding about preserving lastModelError across sparse metadata writes is addressed by carrying lastModelError through mergeSessionMetadata and adding store coverage. Residual risk: static review only; real Cursor ACP error sequencing was not exercised here.

Testing

• Not run (automation; static review only, no PR code executed).

HAPI Bot

[heavygee]

Rebase onto current upstream/main (babysit)

PR was CONFLICTING / ~34 behind (84cd9aa3b tip of main includes companion FCM #803, share-as-image, Codex resume, CreatePlan, etc.).

• New tip: aeeb74bf9 (was f8b9b2c10)
• Conflict: commit feat(hub): emergency-severity push… vs notificationHub / pushNotificationChannel — rerere auto-staged prior resolutions; rest of 17 commits replayed clean
• Preserved: classifier + banner, atTs ack, rawSnippet omit from external notify, lastModelError carry-forward, prior-done same-message fix, no skill_lookup user-turn prepend, ACP title sync
• Verify: bun typecheck (after bun install for upstream html2canvas-pro); classifier / launcher / modelErrorCopy / sessions store+ack / presentation tests green

Force-pushed with lease. Not merging (maintainer only).

[heavygee]

Rebase onto upstream/main @ 83fc9cde3 (babysit)

PR was CONFLICTING / ~21 behind (0.25.x + scratchlist v2, history scroll, etc.).

• New tip: c5108814b (was aeeb74bf9)
• Conflict: shared/src/apiTypes.ts — kept scratchlist v2 schemas and AcknowledgeModelErrorRequestSchema (atTs ack)
• Preserved: classifier/banner, rawSnippet omit from external notify, lastModelError carry-forward, prior-done same-message, ACP title sync, no skill_lookup user-turn prepend
• Verify: bun typecheck; classifier / launcher / modelErrorCopy / sessions store+ack / presentation green

Force-pushed with lease. Not merging.

[github-actions]

Findings

• [Minor] Gate “safe to retry” on actual Bridge eligibility — transient describes the provider failure, but Bridge is separately blocked for non-bridgeable, retry-failed, superseded, or already-bridged events. External push/Telegram/FCM copy can therefore recommend retrying while the banner and REST/RPC gates reject the same action. Evidence hub/src/notifications/modelErrorCopy.ts:59.
  Suggested fix:

  const bridgeEligible = lastModelError.transient
      && lastModelError.bridgeable !== false
      && !lastModelError.retriedAndFailed
      && !lastModelError.supersededByUserTurn
      && lastModelError.bridgedForEventId !== lastModelError.eventId
  
  if (notification.bridgeEligible) {
      lines.push(`(transient - safe to retry)`)
  }

Questions

• None.

Summary

Review mode: follow-up after new commits

The previously reported Minor remains in the full latest diff. No additional findings met the 80% confidence threshold.

Testing

• Not run locally (automation review; PR code was not executed).
• Static git diff --check: passed.
• GitHub checks at review time: Android build, integration, iOS app/package tests, fixture drift gate, and test jobs passed; pr-review was in progress.
• Add copy tests for transient events that are non-bridgeable, retry-failed, superseded, or already bridged; each should omit “safe to retry.”

HAPI Bot

[github-actions]

[MINOR] Gate “safe to retry” on actual Bridge eligibility

transient only classifies the provider failure. The same metadata can still have bridgeable === false, retriedAndFailed, supersededByUserTurn, or bridgedForEventId === eventId; in those cases the web/REST/RPC path rejects Bridge, but every external channel still tells the operator it is safe to retry.

Carry a derived eligibility bit into both notification construction sites and gate this line on it:

const bridgeEligible = lastModelError.transient
    && lastModelError.bridgeable !== false
    && !lastModelError.retriedAndFailed
    && !lastModelError.supersededByUserTurn
    && lastModelError.bridgedForEventId !== lastModelError.eventId

if (notification.bridgeEligible) {
    lines.push(`(transient - safe to retry)`)
}