Skip to content

Cursor chat: surface SDK errors, sync permission mode across clients, consolidate mobile approvals#716

Merged
arul28 merged 4 commits into
mainfrom
ade/claude-auth-login-e0ba903e
Jul 7, 2026
Merged

Cursor chat: surface SDK errors, sync permission mode across clients, consolidate mobile approvals#716
arul28 merged 4 commits into
mainfrom
ade/claude-auth-login-e0ba903e

Conversation

@arul28

@arul28 arul28 commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Summary

Bundles the full lane per request — two workstreams:

1. Cursor chat reliability + cross-client permission-mode sync (desktop + CLI + iOS)

  • Real Cursor errors surfaced. A mid-run Cursor SDK failure (e.g. an NGHTTP2_INTERNAL_ERROR transport reset) previously showed only the generic "Cursor SDK run failed." — the real reason lives only in the SDK's run store. The worker now reads that errorCode on a terminal ERROR and renders Cursor run failed: <code>, classifies transport-class failures as errorInfo.category="network" (so the renderer can offer retry), and logs agent_chat.cursor_sdk_run_error. No auto-retry (a resend could duplicate side effects like a push/PR).
  • Mode changes broadcast live. Changing a chat's permission/mode from one client (e.g. iOS) applied on the runtime but left other clients' composers stale until the turn ended. updateSession now emits a session_meta_updated carrying the permission/interaction/cursor-mode fields (incl. recomputed cursorModeSnapshot); desktop, TUI, and iOS all patch their composer on receipt.

2. iOS Work polish

  • Composer no longer blanks its model/permission controls mid-session (coalesce + latch the summary; cache merges instead of replacing, so a push/deeplink nav reset can't drop the open session's summary).
  • Consolidated pending-input strip pinned above the composer ("Request 1 of N" + Accept-all sweep) replaces the scattered per-request inline approval cards.

3. Sync / lane-naming (concurrent-session work, bundled per request)

  • Persist peer app provenance (appVersion/appBuild/bundleIdentifier) in device metadata.
  • Lane-name heuristic excludes "login history"; auto-title prefers the configured title model before the requested composer model.

Testing

  • /quality (dual-track) clean: 0 Blocker/High/Medium; 2 behavior-preserving cleanups applied.
  • /test: desktop changed-file tests 496 pass; ade-cli 1,506 pass; TUI 814 pass; iOS build + key tests green (incl. a new cacheChatSummaries merge regression test).
  • Docs updated across chat + sync-and-multi-device feature docs.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Improved live session updates so permission, interaction, and cursor settings refresh immediately across chat and composer views.
    • Added richer lane naming behavior with better fallback handling and clearer retry results.
    • Added support for showing more app/device identity details during sync.
  • Bug Fixes

    • Fixed lane naming so “login history” no longer gets misclassified.
    • Improved error reporting for failed runs, including clearer network-related failures.
    • Pending approvals and questions now display in a consolidated, easier-to-use strip without duplicating timeline entries.

Greptile Summary

This PR improves Cursor chat reliability, cross-client mode sync, and mobile chat controls. The main changes are:

  • Surfaces Cursor SDK run error details and network classification.
  • Broadcasts session mode and Cursor config updates to desktop, TUI, and iOS clients.
  • Keeps iOS chat summaries available during partial refreshes.
  • Consolidates iOS pending approvals into one composer strip.
  • Adds peer app provenance and lane naming refinements.

Confidence Score: 4/5

The iOS Cursor summary merge can clear live composer state from an omitted field.

Explicit clear handling is improved across the changed sync path, but the new wholesale merge treats absent cached Cursor fields as clears.

apps/ios/ADE/Models/RemoteModels.swift

T-Rex T-Rex Logs

What T-Rex did

  • Verification in the Linux sandbox showed that Swift and Xcode tooling were unavailable, blocking the Swift decoding and merge path for absent cursor fields.
  • Saved a minimal Swift harness that mirrors the repository cursor fields and merge statements to enable reruns in an environment where Swift is available.
  • Ran the desktop Vitest suite; the run completed with Exit status 0 and a complete test summary across 3 files and 496 tests in 17.07s.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
apps/ios/ADE/Models/RemoteModels.swift Adds explicit null handling for Cursor mode and config updates, but the live-summary merge can over-apply nil cursor fields from cached summaries.
apps/desktop/src/main/services/chat/agentChatService.ts Emits transient session metadata updates for cross-client mode and Cursor config changes.
apps/desktop/src/renderer/components/chat/AgentChatPane.tsx Applies session metadata updates to the desktop composer, including Cursor mode and config clears.
apps/ade-cli/src/tuiClient/app.tsx Applies session metadata updates to the TUI composer without echoing local commits.

Comments Outside Diff (2)

  1. apps/desktop/src/main/services/chat/cursorSdkWorker.ts, line 615-620 (link)

    P2 Error Event Reorders Stream

    This loop withholds the first terminal ERROR status but keeps forwarding any later stream events before posting the held error after wait(). If the SDK emits cleanup/final status events after the error status, the chat bridge observes those events before the failure and can update the transcript or run state in the wrong order.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/desktop/src/main/services/chat/cursorSdkWorker.ts
    Line: 615-620
    
    Comment:
    **Error Event Reorders Stream**
    
    This loop withholds the first terminal `ERROR` status but keeps forwarding any later stream events before posting the held error after `wait()`. If the SDK emits cleanup/final status events after the error status, the chat bridge observes those events before the failure and can update the transcript or run state in the wrong order.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Claude Code

  2. apps/ios/ADE/Models/RemoteModels.swift, line 978-980 (link)

    P1 Absent Cursor Fields Clear State

    When a cached summary comes from a payload that omits cursorModeId or cursorConfigValues, those optionals decode as nil. This merge then copies those nils into the live summary, so a partial refresh can clear the open iOS composer's Cursor mode or config even though no clear event was received.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/ios/ADE/Models/RemoteModels.swift
    Line: 978-980
    
    Comment:
    **Absent Cursor Fields Clear State**
    
    When a cached summary comes from a payload that omits `cursorModeId` or `cursorConfigValues`, those optionals decode as `nil`. This merge then copies those nils into the live summary, so a partial refresh can clear the open iOS composer's Cursor mode or config even though no clear event was received.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Claude Code

Fix All in Claude Code

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/ios/ADE/Models/RemoteModels.swift:978-980
**Absent Cursor Fields Clear State**

When a cached summary comes from a payload that omits `cursorModeId` or `cursorConfigValues`, those optionals decode as `nil`. This merge then copies those nils into the live summary, so a partial refresh can clear the open iOS composer's Cursor mode or config even though no clear event was received.

Reviews (4): Last reviewed commit: "iOS: stateless cursor mode/config clear ..." | Re-trigger Greptile

…ents, consolidate mobile approvals

Chat / Cursor SDK (desktop + CLI + iOS):
- Surface the real Cursor run failure reason: read the run store's errorCode on
  a terminal ERROR and render "Cursor run failed: <code>" instead of the generic
  "Cursor SDK run failed."; classify NGHTTP2/ECONNRESET-class transport failures
  as errorInfo.category="network" and log agent_chat.cursor_sdk_run_error.
- Broadcast permission/mode changes to every client: updateSession now emits a
  session_meta_updated carrying the permission/interaction/cursorMode fields
  (incl. recomputed cursorModeSnapshot) so desktop, TUI, and iOS composers update
  live when any client re-modes a session, instead of going stale until a turn
  ends. Desktop (AgentChatPane), TUI (tuiClient/app.tsx), and iOS all patch on
  receipt.

iOS Work:
- Composer no longer blanks its model/permission controls mid-session: the
  composer summary is coalesced + latched and the summary cache merges instead of
  replacing, so a push/deeplink nav @State reset can't drop the open session's
  summary.
- Consolidated pending-input strip pinned above the composer ("Request 1 of N" +
  Accept-all sweep) replaces the scattered per-request inline approval cards.

Sync / lane naming (concurrent-session work bundled per request):
- Persist peer app provenance (appVersion/appBuild/bundleIdentifier) in device
  metadata.
- Lane-name heuristic excludes "login history"; auto-title prefers the configured
  title model before the requested composer model.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@vercel

vercel Bot commented Jul 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview Jul 7, 2026 6:11am

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds application provenance metadata (appVersion/appBuild/bundleIdentifier) to sync peer handshakes, adds structured logging and retry-based orchestration to lane-name suggestion/auto-rename flows, propagates session permission/mode changes via extended session_meta_updated events across desktop and iOS, classifies Cursor SDK transport errors with a new errorCode field, and replaces iOS per-type pending-input timeline cards with a consolidated pending-input strip supporting optimistic answering and accept-all sweeping.

Changes

App identity metadata propagation

Layer / File(s) Summary
Peer provenance fields
apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts, .../deviceRegistryService.ts, .../syncHostService.ts, .../syncService.test.ts, apps/desktop/src/shared/types/sync.ts, apps/ios/ADE/Services/SyncService.swift
Adds and normalizes optional appVersion/appBuild/bundleIdentifier fields through peer metadata parsing, storage, type definitions, and iOS handshake construction, with a new backend test.

Lane and session naming suggestions

Layer / File(s) Summary
Login-history phrase exclusion
apps/desktop/src/shared/laneNameFallback.ts, .../laneNameFallback.test.ts, apps/ios/ADE/Views/Work/WorkNewChatScreen.swift
Refines login-detection regexes so "login history" no longer triggers login-related naming.
lanes.suggestName logging
apps/ade-cli/src/services/sync/syncRemoteCommandService.ts, .../syncRemoteCommandService.test.ts
Adds warn/info logging for fallback usage, empty suggestions, and successful suggestions.
Auto-title model ordering
apps/desktop/src/main/services/chat/agentChatService.ts, .../agentChatService.test.ts
Reorders auto-title candidates to prefer the configured title model over the requested model.
iOS auto-lane AI rename orchestration
apps/ios/ADE/Views/Work/WorkNewChatScreen.swift, .../Hub/HubComposerDrawer.swift, apps/ios/ADETests/ADETests.swift
Introduces WorkAutoLaneNamingClient and workRunAutoLaneAiRename to centralize suggestion retries and renaming, used by both New Chat and Hub composer flows.

session_meta_updated mode propagation

Layer / File(s) Summary
Desktop backend broadcast
apps/desktop/src/main/services/chat/agentChatService.ts, .../agentChatService.test.ts
Emits a transient session_meta_updated envelope with permission/interaction/cursor fields when mode changes are detected.
Shared type and consumers
apps/desktop/src/shared/types/chat.ts, .../renderer/components/chat/AgentChatPane.tsx, apps/ade-cli/src/tuiClient/app.tsx
Extends the event type with mode fields and applies them to renderer/TUI composer and session state.
iOS mode update model
apps/ios/ADE/Models/RemoteModels.swift
Adds AgentChatSessionMetaModeUpdate decoding and apply/merge helpers on AgentChatSessionSummary.
iOS SyncService caching
apps/ios/ADE/Services/SyncService.swift, apps/ios/ADE/Views/Work/WorkRootScreen+Actions.swift
Merges partial summary refreshes instead of replacing the cache and tracks cleared cursor-mode sessions.
iOS composer latching
apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift, apps/ios/ADETests/ADETests.swift
Latches the last known chat summary and reconciles cached mode fields before live subscription.

Cursor SDK transport error classification

Layer / File(s) Summary
Protocol/pool errorCode plumbing
apps/desktop/src/main/services/chat/cursorSdkProtocol.ts, .../cursorSdkPool.ts
Adds errorCode field and isCursorSdkTransportErrorText helper, forwarding errorCode through run_result.
Worker error recovery
apps/desktop/src/main/services/chat/cursorSdkWorker.ts
Adds terminal ERROR detection and readRunErrorCode recovery, enriching run_result with the recovered error code.
Mapper and service consumption
.../cursorSdkEventMapper.ts, .../cursorSdkEventMapper.test.ts, apps/desktop/src/main/services/chat/agentChatService.ts
Builds error messages/errorInfo.category from adeErrorCode and logs errorCode on run results.

iOS consolidated pending input strip

Layer / File(s) Summary
Pending-input state
apps/ios/ADE/Views/Work/WorkChatSessionView.swift
Adds optimistic-answer tracking, queue signature, sweepable-input detection, and simplified blocking-input handling.
Optimistic dispatch and accept-all
apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift
Adds dispatchPendingInputAnswer, reconciliation, and acceptAllPendingInputs.
Strip UI
apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift
Renders a single consolidated pending-input strip per pending type.
Timeline cleanup and tests
apps/ios/ADE/Views/Work/WorkTimelineHelpers.swift, apps/ios/ADETests/ADETests.swift
Removes inline pending-input timeline entries and updates tests accordingly.

Estimated code review effort: 4 (Complex) | ~75 minutes

Possibly related PRs

  • arul28/ADE#591: Both PRs modify the lanes.suggestName sync remote command handler and its tests, adding/using fallbackName and logging behavior.
  • arul28/ADE#698: Both PRs update iOS background auto-lane AI naming, creating lanes with a deterministic fallback before asynchronously suggesting/renaming.

Suggested labels: desktop, ios

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the three main themes of the PR: Cursor SDK errors, cross-client permission sync, and mobile approval consolidation.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade/claude-auth-login-e0ba903e

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@arul28 arul28 changed the title Claude Auth Login Cursor chat: surface SDK errors, sync permission mode across clients, consolidate mobile approvals Jul 7, 2026
@arul28

arul28 commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

@copilot review but do not make fixes

Comment thread apps/desktop/src/main/services/chat/agentChatService.ts
Comment thread apps/ios/ADE/Models/RemoteModels.swift
…meta_updated

Addresses two Greptile P1 findings on the mode-broadcast path:

- Config values dropped: a cursorConfigValues-only updateSession emitted
  session_meta_updated without the new config (and the snapshot recompute is
  skipped for config-only changes), so other clients ran the next Cursor turn
  with stale options. The event now carries cursorConfigValues (incl. explicit
  null on clear); desktop AgentChatPane and iOS patch it, with an explicit
  value taking precedence over snapshot-derived config.

- Explicit cursorModeId: null clear ignored on iOS: decodeIfPresent collapsed
  absent-key and explicit-null, and applyModeUpdate skipped nil, so a host
  mode-clear never reached the composer. iOS now distinguishes present-but-null
  (via container.contains) from absent and applies the clear, while absent-key
  still means "no change". Scoped to cursorModeId only (interactionMode maps to
  a default, not a distinct cleared state).

Adds testAgentChatMetaModeUpdateAppliesCursorConfigAndExplicitClear.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@arul28

arul28 commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Comment thread apps/ade-cli/src/tuiClient/app.tsx Outdated
Comment thread apps/desktop/src/renderer/components/chat/AgentChatPane.tsx Outdated
Comment thread apps/ios/ADE/Models/RemoteModels.swift Outdated
…pdated consumer

Sweeps the remaining instances of two bug classes across all clients that
re-seed composer state from session_meta_updated:

- cursorConfigValues now applied by the TUI handler (app.tsx) too, matching
  desktop and iOS, so a config-only change reaches every composer live.
- Explicit cursorModeId:null (a mode clear) now propagates end to end:
  desktop AgentChatPane keys on "cursorModeId" in meta (present-null clears,
  absent = no change) instead of a stale ?? fallback; the TUI applies the same
  present-vs-absent distinction; iOS threads the cleared intent through the
  open-view reconcile (SyncService cursorModeClearedSessionIds +
  mergeModeFields(cursorModeIdCleared:)) so the clear reaches the live
  chatSummary, not just the cache.

Absent-key still means no-change on every hop. Extends the iOS mode-update
test to assert the clear reaches the live summary.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@arul28

arul28 commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@cursor

cursor Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1bff4c9d46

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/ios/ADE/Models/RemoteModels.swift Outdated
cursorModeIdWasCleared = false
}
cursorModeSnapshot = try c.decodeIfPresent(RemoteJSONValue.self, forKey: .cursorModeSnapshot)
cursorConfigValues = try c.decodeIfPresent([String: RemoteJSONValue].self, forKey: .cursorConfigValues)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle null cursorConfigValues clears

When another client clears Cursor config overrides, the host emits session_meta_updated with cursorConfigValues: null (the desktop emitter does this when the update explicitly clears the normalized values). This decode uses decodeIfPresent, so an explicit null is indistinguishable from an absent key; hasAnyField stays false for config-only clears and applyModeUpdate never removes stale cursorConfigValues. In that case the iOS composer/cache keeps showing the old Cursor config until a full refresh or another non-null config update arrives.

Useful? React with 👍 / 👎.

Comment thread apps/ios/ADE/Services/SyncService.swift
Comment thread apps/ios/ADE/Models/RemoteModels.swift Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts (1)

145-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate provenance-parsing logic across two sync entry points.

The same appVersion/appBuild/bundleIdentifier extraction pattern (plus the pre-existing dbVersionBySite/capabilities normalization) is now duplicated in normalizePeerMetadata here and in parseHelloPayload in apps/ade-cli/src/services/sync/syncHostService.ts (Lines 977-979). Consider extracting a shared normalizePeerAppProvenance(record) helper so future peer-metadata fields only need to be added in one place.

♻️ Example shared helper
export function normalizePeerAppProvenance(record: Record<string, unknown>): {
  appVersion?: string;
  appBuild?: string;
  bundleIdentifier?: string;
} {
  const appVersion = optionalString(record.appVersion);
  const appBuild = optionalString(record.appBuild);
  const bundleIdentifier = optionalString(record.bundleIdentifier);
  return {
    ...(appVersion ? { appVersion } : {}),
    ...(appBuild ? { appBuild } : {}),
    ...(bundleIdentifier ? { bundleIdentifier } : {}),
  };
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts` around
lines 145 - 173, The appVersion/appBuild/bundleIdentifier parsing is duplicated
between normalizePeerMetadata and parseHelloPayload, so add a shared helper for
peer provenance normalization and reuse it in both places. Extract the
optionalString-based extraction into a helper like normalizePeerAppProvenance,
keep the existing dbVersionBySite/capabilities normalization where it belongs,
and update both normalizePeerMetadata and syncHostService's parseHelloPayload to
call the shared helper so future peer-metadata fields are maintained in one
place.
apps/desktop/src/main/services/chat/cursorSdkWorker.ts (1)

141-170: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider preferring the SDK's own error taxonomy over text matching for thrown errors.

For the thrown-error path (as opposed to the stored errorCode string path, where no structured object exists), @cursor/sdk's CursorAgentError already exposes isRetryable and a NetworkError subclass. Falling back to isCursorSdkTransportErrorText(detail) string matching only when those aren't checked first means this classification stays coupled to specific wire-error substrings ("nghttp2", "econnreset", etc.) that could shift across SDK releases, whereas error.isRetryable/error instanceof NetworkError is the documented, stable signal for this.

♻️ Possible approach
+  if (error instanceof CursorAgentError && error.isRetryable) {
+    return { error: errorMessage(error), errorCode: error.code ?? "network" };
+  }
   const detail = errorMessage(error);
   if (isCursorSdkTransportErrorText(detail) || isCursorSdkTransportErrorText(errorCode(error))) {
     return { error: detail, errorCode: "network" };
   }

Please confirm CursorAgentError/isRetryable/NetworkError are exported and usable from the pinned @cursor/sdk@1.0.13 before adopting this.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/main/services/chat/cursorSdkWorker.ts` around lines 141 -
170, The thrown-error path in classifyWorkerError currently relies on text
matching for transport failures; prefer the SDK’s structured error types first.
Update cursorSdkWorker’s classifyWorkerError to check the `@cursor/sdk` error
taxonomy on the error object itself, using CursorAgentError’s isRetryable and
the NetworkError subclass before falling back to isCursorSdkTransportErrorText
or errorCode string matching. Keep the existing agent_busy and rate_limited
handling, but make sure the network classification is driven by the exported SDK
symbols rather than wire-error substrings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/ios/ADE/Views/Work/WorkChatSessionView`+Actions.swift:
- Around line 985-1000: acceptAllPendingInputs should stop sweeping if the
primary acceptForSession fails. Update the acceptAllPendingInputs flow in
WorkChatSessionView+Actions so the loop over acceptAllSweepableInputs only runs
after sendPendingInputDecision(primary, decision: .acceptForSession) succeeds;
if that call fails or returns unsuccessfully, exit early and do not send .accept
for the remaining items. Use the existing acceptAllPendingInputs,
sendPendingInputDecision, and optimistic state checks to keep the sweep
consistent with the primary decision.
- Around line 953-967: The rollback check in dispatchPendingInputAnswer is
relying on the shared errorMessage state, which can be changed by unrelated chat
actions. Update dispatchPendingInputAnswer to use a per-action success/failure
result from the awaited op or runSessionAction instead of inspecting
errorMessage, and only remove the itemId from optimisticallyAnsweredInputIds
when that specific action reports failure. Use the dispatchPendingInputAnswer
and runSessionAction symbols to locate the flow and keep the answer handler
logic isolated from global chat errors.

---

Nitpick comments:
In `@apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts`:
- Around line 145-173: The appVersion/appBuild/bundleIdentifier parsing is
duplicated between normalizePeerMetadata and parseHelloPayload, so add a shared
helper for peer provenance normalization and reuse it in both places. Extract
the optionalString-based extraction into a helper like
normalizePeerAppProvenance, keep the existing dbVersionBySite/capabilities
normalization where it belongs, and update both normalizePeerMetadata and
syncHostService's parseHelloPayload to call the shared helper so future
peer-metadata fields are maintained in one place.

In `@apps/desktop/src/main/services/chat/cursorSdkWorker.ts`:
- Around line 141-170: The thrown-error path in classifyWorkerError currently
relies on text matching for transport failures; prefer the SDK’s structured
error types first. Update cursorSdkWorker’s classifyWorkerError to check the
`@cursor/sdk` error taxonomy on the error object itself, using CursorAgentError’s
isRetryable and the NetworkError subclass before falling back to
isCursorSdkTransportErrorText or errorCode string matching. Keep the existing
agent_busy and rate_limited handling, but make sure the network classification
is driven by the exported SDK symbols rather than wire-error substrings.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c0c7e8d3-a80e-4201-bb70-7187e8f1f3f5

📥 Commits

Reviewing files that changed from the base of the PR and between e78308f and 1bff4c9.

📒 Files selected for processing (30)
  • apps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.ts
  • apps/ade-cli/src/services/sync/deviceRegistryService.ts
  • apps/ade-cli/src/services/sync/syncHostService.ts
  • apps/ade-cli/src/services/sync/syncRemoteCommandService.test.ts
  • apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
  • apps/ade-cli/src/services/sync/syncService.test.ts
  • apps/ade-cli/src/tuiClient/app.tsx
  • apps/desktop/src/main/services/chat/agentChatService.test.ts
  • apps/desktop/src/main/services/chat/agentChatService.ts
  • apps/desktop/src/main/services/chat/cursorSdkEventMapper.test.ts
  • apps/desktop/src/main/services/chat/cursorSdkEventMapper.ts
  • apps/desktop/src/main/services/chat/cursorSdkPool.ts
  • apps/desktop/src/main/services/chat/cursorSdkProtocol.ts
  • apps/desktop/src/main/services/chat/cursorSdkWorker.ts
  • apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
  • apps/desktop/src/shared/laneNameFallback.test.ts
  • apps/desktop/src/shared/laneNameFallback.ts
  • apps/desktop/src/shared/types/chat.ts
  • apps/desktop/src/shared/types/sync.ts
  • apps/ios/ADE/Models/RemoteModels.swift
  • apps/ios/ADE/Services/SyncService.swift
  • apps/ios/ADE/Views/Hub/HubComposerDrawer.swift
  • apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift
  • apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift
  • apps/ios/ADE/Views/Work/WorkChatSessionView.swift
  • apps/ios/ADE/Views/Work/WorkNewChatScreen.swift
  • apps/ios/ADE/Views/Work/WorkRootScreen+Actions.swift
  • apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift
  • apps/ios/ADE/Views/Work/WorkTimelineHelpers.swift
  • apps/ios/ADETests/ADETests.swift

Comment on lines +953 to +967
@MainActor
func dispatchPendingInputAnswer(
itemId: String,
_ op: @escaping @MainActor () async -> Void
) async {
optimisticallyAnsweredInputIds.insert(itemId)
await runSessionAction(op)
// Every answer handler (`approveRequest`, `respondToPermission`,
// `submitQuestionAnswers`, `respondToQuestion`, `declineQuestion`) resets
// `errorMessage` to nil on success and sets a message on failure, so a
// non-nil value here means THIS command failed — roll the hide back.
if errorMessage != nil {
optimisticallyAnsweredInputIds.remove(itemId)
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the five answer handlers reset errorMessage deterministically.
rg -n -A 15 'func (approveRequest|respondToPermission|submitQuestionAnswers|respondToQuestion|declineQuestion)\s*\(' apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift

Repository: arul28/ADE

Length of output: 148


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the relevant files and symbols.
git ls-files 'apps/ios/ADE/Views/Work/*' | sed -n '1,120p'
rg -n 'approveRequest|respondToPermission|submitQuestionAnswers|respondToQuestion|declineQuestion|errorMessage|optimisticallyAnsweredInputIds|dispatchPendingInputAnswer' apps/ios/ADE/Views/Work -S

Repository: arul28/ADE

Length of output: 18337


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the relevant files under apps/ios and inspect their outlines.
fd -a 'WorkSessionDestinationView.swift|WorkChatSessionView\+Actions.swift' apps/ios
printf '\n---\n'
git ls-files 'apps/ios/**/*.swift' | rg 'WorkSessionDestinationView\.swift|WorkChatSessionView\+Actions\.swift'
printf '\n---\n'
for f in $(git ls-files 'apps/ios/**/*.swift' | rg 'WorkSessionDestinationView\.swift|WorkChatSessionView\+Actions\.swift'); do
  echo "FILE: $f"
  wc -l "$f"
done

Repository: arul28/ADE

Length of output: 678


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift"
chat="apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift"

echo '--- ANSWER HANDLERS ---'
sed -n '80,390p' "$file" | cat -n

echo '--- ROLLBACK SITE ---'
sed -n '950,970p' "$chat" | cat -n

echo '--- CALL SITES ---'
sed -n '1000,1020p' "$chat" | cat -n

Repository: arul28/ADE

Length of output: 15171


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -A 25 -B 10 'func runSessionAction|runSessionAction\(' apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift -S

Repository: arul28/ADE

Length of output: 5933


Use a per-action result for dispatchPendingInputAnswer. errorMessage is shared with other chat actions, so an unrelated error during the awaited action can leave it non-nil and roll back a successful answer. Return success/failure explicitly instead of reading the global binding.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/ios/ADE/Views/Work/WorkChatSessionView`+Actions.swift around lines 953 -
967, The rollback check in dispatchPendingInputAnswer is relying on the shared
errorMessage state, which can be changed by unrelated chat actions. Update
dispatchPendingInputAnswer to use a per-action success/failure result from the
awaited op or runSessionAction instead of inspecting errorMessage, and only
remove the itemId from optimisticallyAnsweredInputIds when that specific action
reports failure. Use the dispatchPendingInputAnswer and runSessionAction symbols
to locate the flow and keep the answer handler logic isolated from global chat
errors.

Comment thread apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift
Review follow-ups (Codex P2, Greptile, CodeRabbit) on the mode-broadcast + approval work:

- Cursor clear handling is now stateless. Removed the cursorModeClearedSessionIds
  marker (and the mergeModeFields cleared param) that could go stale — a bulk
  merge caching a host-restored mode left the marker set, wrongly re-clearing the
  composer. Clears are now applied once at receipt: an explicit cursorModeId/
  cursorConfigValues null nulls the cached field, and the open view mirrors the
  cache (nil included) on reconcile. A restored value can never be re-cleared
  because no "was cleared" state is remembered.
- cursorConfigValues: null clears now decode via container.contains (present-null
  vs absent), matching cursorModeId; hasAnyField/applyModeUpdate honor the clear.
  Desktop + TUI already treated present-null as a clear (no change needed).
- Accept-all sweep: sendPendingInputDecision/dispatchPendingInputAnswer now return
  an explicit per-action success (captured immediately after the handler's write,
  not from shared errorMessage), and acceptAllPendingInputs stops early if the
  primary acceptForSession fails instead of sweeping the rest.

Absent-key still means no-change on every hop. Extends the iOS mode-update test
with clear-reaches-live-summary and restored-value-not-re-cleared assertions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@arul28

arul28 commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 8b1d5a64b7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@arul28 arul28 merged commit 8d514f1 into main Jul 7, 2026
33 checks passed
@arul28 arul28 deleted the ade/claude-auth-login-e0ba903e branch July 7, 2026 06:27
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