Skip to content

Fix iOS artifact failure messaging - #9961

Merged
azooz2003-bit merged 17 commits into
mainfrom
issue-9899-files-error-message
Aug 13, 2026
Merged

azooz2003-bit merged 17 commits into
mainfrom
issue-9899-files-error-message

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Aug 11, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #9899

Summary:

  • preserve typed artifact failures from Mac filesystem and RPC handlers through transport, transfer validation, cache and temporary storage, and every iOS file viewer
  • reserve “Mac unreachable” for a confirmed closed control connection
  • distinguish invalid requests; missing sessions, terminals, workspaces, repositories, and files; authorization and permission failures; wrong filesystem types; read and mutation races; unsupported, damaged, and failed media previews; malformed and interrupted transfers; timeouts and reconnect states; security, authentication, and account failures; local storage failures; size limits; and unknown load failures
  • validate ordered chunks, stable sizes, EOF, and stat-to-transfer consistency
  • reject special files without blocking, including FIFOs
  • localize every user-facing outcome in English and Japanese

Tests:

The first commit is test-only and fails on the prior behavior. The implementation commits preserve typed failures end to end, scope storage classification, keep terminal failures visible, and isolate UI-test controls in the DEBUG-only preview entrypoint.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The change separates artifact loading failures from Mac reachability failures. It adds missing-session and load-failure states, updates artifact and terminal gallery UI handling, adds localized messages, and expands unit and UI-test coverage.

Changes

Artifact failure handling

Layer / File(s) Summary
Artifact error contract and classification
Packages/Shared/CmuxAgentChat/Sources/..., Packages/iOS/CmuxAgentChatUI/Sources/..., Packages/iOS/CmuxMobileShell/Sources/..., Packages/iOS/CmuxMobileShell/Tests/...
The artifact pipeline adds ChatArtifactError.loadFailed, classifies transport and RPC errors, and reports invalid or incomplete data as load failures.
Artifact viewer states and presentation
Packages/iOS/CmuxAgentChatUI/Sources/..., Packages/iOS/CmuxAgentChatUI/Tests/...
The viewer distinguishes missing sessions, load failures, and unreachable Macs. It applies retry rules and localized messages for each state.
Terminal gallery failure presentation and test fixtures
Packages/iOS/CmuxMobileShellUI/Sources/..., Packages/iOS/CmuxMobileShellUI/Tests/..., Packages/iOS/CmuxAgentChatUI/Sources/..., ios/cmuxUITests/cmuxUITests.swift
The terminal gallery stores categorized failures and renders category-specific titles, icons, messages, and retry behavior. The demo screen and UI tests support injected artifact failures and stable attachment controls.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MobileChatEventSource
  participant MobileChatArtifactFailureClassifier
  participant ChatArtifactViewerModel
  participant TerminalArtifactFilesSheet
  MobileChatEventSource->>MobileChatArtifactFailureClassifier: classify artifact error
  MobileChatArtifactFailureClassifier->>ChatArtifactViewerModel: return ChatArtifactError
  MobileChatArtifactFailureClassifier->>TerminalArtifactFilesSheet: provide categorized failure
  ChatArtifactViewerModel->>ChatArtifactViewerModel: render state and retry policy
  TerminalArtifactFilesSheet->>TerminalArtifactFilesSheet: render title, icon, message, and retry policy
Loading

Suggested reviewers: lawrencecchen


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Cmux No Test Or Debug Seam In Production Source ❌ Error AgentChatDemoScreen.swift adds private makeArtifactLoader and CMUX_UITEST_AGENT_CHAT_ARTIFACT_FAILURE fault injection under #if DEBUG; this is test-only scaffolding in production Sources. Move the failure-injection facility to the UI-test support target or a dedicated debug/testing file or folder, and keep AgentChatDemoScreen free of the test seam; use @testable import for internal-state observation.
Docstring Coverage ⚠️ Warning Docstring coverage is 6.90% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (23 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue [#9899] by reserving “Mac unreachable” for closed connections and distinguishing artifact-specific failures.
Out of Scope Changes check ✅ Passed The accessibility updates and UI test hooks support the artifact failure behavior and regression coverage described by the objectives.
Cmux Swift Actor Isolation ✅ Passed The production diff adds only a pure Sendable classifier; MobileChatEventSource remains an actor, and changed UI types are SwiftUI/MainActor-bound. No new isolated service protocol or shared mutabl...
Cmux Swift Blocking Runtime ✅ Passed Production Swift changes add no blocking or timing primitives. The only new polling and RunLoop delay are in ios/cmuxUITests test scaffolding, which the rule allows.
Cmux Browser Automation Off-Main ✅ Passed The PR changes only iOS artifact handling and attachment UI tests; it does not modify TerminalController.swift, ControlCommandExecutionPolicy.swift, or browser socket automation commands.
Cmux Expensive Synchronous Load ✅ Passed The PR diff only changes artifact error mapping, UI states, writers, and test injection; no added agent-history loader, broad scan, large JSON parse, or main-actor synchronous history load appears.
Cmux Cache Substitution Correctness ✅ Passed The production diff changes artifact error classification and UI state only; the cache writer still serializes stream data, with no fresh-read replacement in persistence, history, undo, or snapshot...
Cmux No Hacky Sleeps ✅ Passed The PR changes only Swift sources/tests and localization data; it adds no TypeScript, JavaScript, shell, or build/runtime script delays covered by this rule.
Cmux Algorithmic Complexity ✅ Passed The production diff adds error classification and UI state handling only; it adds no scalable nested scan, per-target rescan, repeated sort/filter, or slower batch algorithm. Added loops are test-o...
Cmux Swift Concurrency ✅ Passed The PR adds no DispatchQueue, Combine, completion-handler, or fire-and-forget Task pattern; new async code is test code, and the SwiftUI retry Task is unchanged from origin/main.
Cmux Swift @Concurrent ✅ Passed The diff adds no production @concurrent or nonisolated async work; changed async code remains existing actor/UI-bound code, and the new failure classifier is synchronous.
Cmux Swift Package Boundaries ✅ Passed All production Swift changes are inside existing SwiftPM package Sources; classifier logic has package tests, and the only app-target change is UI test code.
Cmux Swiftpm Lockfiles ✅ Passed The full PR diff contains no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project changes; therefore the SwiftPM lockfile requirements do not apply.
Cmux Swift Logging ✅ Passed The complete PR diff adds no print, debugPrint, dump, NSLog, Logger, stdout, or stderr logging; FileHandle changes are artifact writing, not diagnostics.
Cmux User-Facing Error Privacy ✅ Passed The production diff exposes only generic localized artifact messages and preserves typed errors; it does not display upstream messages, provider details, IDs, credentials, payloads, or environment-...
Cmux Full Internationalization ✅ Passed All new production UI copy uses localized APIs, and both touched catalogs provide translated en and ja entries for every new or changed key.
Cmux Swiftui State Layout ✅ Passed The PR adds no new ObservableObject/@published wrappers, GeometryReader, lazy-row store references, or render-time writes; gallery state changes stay in async loaders and failure helpers are render...
Cmux Architecture Rethink ✅ Passed PASS: The diff adds typed state transitions and a centralized failure classifier; production code adds no timing, locks, observers, mutable side channels, or split lifecycle owners, and polling is...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The full PR diff adds no NSWindow, NSPanel, NSWindowController, Window, WindowGroup, or close-shortcut ownership code; all changes are shared/iOS artifact code or tests.
Cmux Source Artifacts ✅ Passed All 21 supplied changed paths are intentional Swift source, tests, UI tests, or localization catalogs; no prohibited scratch, cache, build-output, log, or copied-artifact path appears.
Cmux No Ambient Global State ✅ Passed The production Swift diff adds no file-scope functions, mutable globals, or singleton; the new classifier uses an instance method, and loader state is private instance state on AgentChatDemoScreen.
Title check ✅ Passed The title clearly and concisely describes the primary change to iOS artifact failure messaging.
Description check ✅ Passed The description explains the scope and rationale and lists targeted tests and results, with only template sections omitted.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-9899-files-error-message

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.

@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: 1

🤖 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
`@Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Artifacts/ChatArtifactError.swift`:
- Around line 17-20: Update the documentation comment for
ChatArtifactError.loadFailed to describe a general artifact operation failure,
including request, authorization, route, stream, and file-handle failures, when
control-transport closure has not been confirmed. Leave macUnreachable’s
transport-closure contract unchanged.
🪄 Autofix

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: ASSERTIVE

Plan: Pro Plus

Run ID: bf7d8e2d-b27a-43ee-b0d7-76fe5efb65aa

📥 Commits

Reviewing files that changed from the base of the PR and between 61ca64f and eadad94.

📒 Files selected for processing (20)
  • Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Artifacts/ChatArtifactError.swift
  • Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactActionVisibilityPolicy.swift
  • Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactContentCacheWriter.swift
  • Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactTemporaryFileWriter.swift
  • Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactViewerModel.swift
  • Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactViewerRouteView.swift
  • Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Artifacts/ChatArtifactViewerState.swift
  • Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Resources/Localizable.xcstrings
  • Packages/iOS/CmuxAgentChatUI/Tests/CmuxAgentChatUITests/ChatArtifactViewerModelTests.swift
  • Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileArtifactChunkFetchLoop.swift
  • Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileChatArtifactFailureClassifier.swift
  • Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileChatEventSource.swift
  • Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileArtifactChunkFetchLoopTests.swift
  • Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileChatArtifactFailureClassifierTests.swift
  • Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/AgentChatDemoScreen.swift
  • Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/Localizable.xcstrings
  • Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalArtifactFilesSheet+Content.swift
  • Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalArtifactFilesSheet.swift
  • Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalArtifactGalleryFailureTests.swift
  • ios/cmuxUITests/cmuxUITests.swift

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 27203cb. Configure here.

Comment thread ios/cmuxUITests/cmuxUITests.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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ios/cmuxUITests/cmuxUITests.swift (1)

7412-7428: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Replace the fixed run-loop delay with a completion-driven wait.

Line 7424 waits for a fixed 0.12 seconds after every swipe. This does not confirm that the transcript moved or that the target became hittable. On slow UI runs, the helper can retry before scrolling settles. On fast runs, it adds unnecessary delay.

Keep the overall deadline. After each swipe, wait for element.isHittable or a real frame/scroll-position change with a deadline-bounded predicate.

As per coding guidelines: “Tests must await real completion signals or deadline-bounded polls of real predicates rather than fixed-duration waits before assertions.”

🤖 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 `@ios/cmuxUITests/cmuxUITests.swift` around lines 7412 - 7428, Replace the
fixed RunLoop delay in scrollTranscript with a deadline-bounded poll of a real
completion predicate after each table.swipeDown, such as element.isHittable or a
changed frame/scroll position. Preserve the existing overall timeout and return
behavior while ensuring retries occur only after scrolling has progressed or the
target becomes hittable.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@ios/cmuxUITests/cmuxUITests.swift`:
- Around line 7412-7428: Replace the fixed RunLoop delay in scrollTranscript
with a deadline-bounded poll of a real completion predicate after each
table.swipeDown, such as element.isHittable or a changed frame/scroll position.
Preserve the existing overall timeout and return behavior while ensuring retries
occur only after scrolling has progressed or the target becomes hittable.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 91cbe366-9070-4705-9b96-a785e354bb01

📥 Commits

Reviewing files that changed from the base of the PR and between 609fa9d and f69b300.

📒 Files selected for processing (1)
  • ios/cmuxUITests/cmuxUITests.swift

@cursor

cursor Bot commented Aug 13, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

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.

iOS: Files says "Mac unreachable" while the rest of the app is live

1 participant