Skip to content

fix(ios): convert the stamped group id to a workspace id (unbreaks app-host unit tests) - #10825

Closed
austinywang wants to merge 1 commit into
mainfrom
fix/ios-group-anchor-id
Closed

austinywang wants to merge 1 commit into
mainfrom
fix/ios-group-anchor-id

Conversation

@austinywang

@austinywang austinywang commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

CI's app-host unit test shards fail on main's tree at MobileWorkspaceAggregation.swift:285: stamped.anchorWorkspaceID = stamped.id assigns a MobileWorkspaceGroupPreview.ID to a MobileWorkspacePreview.ID, which Xcode 26.3 rejects (cannot assign value of type 'MobileWorkspaceGroupPreview.ID' to type 'MobileWorkspacePreview.ID'). Main's own recent runs skipped those shards, so it went unnoticed and every branch built from main fails them (seen on #10812).

Both ids are string-backed; this converts through rawValue, keeping the documented intent (the namespaced group id as a stable UI row identity, never a workspace capability).

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Bug Fixes
    • Improved workspace grouping behavior by correctly anchoring empty group headers to their associated workspace.

… workspace id

MobileWorkspaceAggregation assigned a MobileWorkspaceGroupPreview.ID to
anchorWorkspaceID (a MobileWorkspacePreview.ID); Xcode 26.3 on CI rejects the
assignment, failing every app-host unit test shard on main and on every branch
built from it. Both ids are string-backed, so convert through rawValue as the
comment already describes (a stable UI row identity, never a capability).

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

coderabbitai Bot commented Aug 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e3cbd690-a035-445a-9086-a535a3191f6f

📥 Commits

Reviewing files that changed from the base of the PR and between 7a28edc and 6fcde84.

📒 Files selected for processing (1)
  • Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The workspace aggregation fallback now constructs anchorWorkspaceID as MobileWorkspacePreview.ID from the namespaced group ID raw value when no live anchor workspace exists.

Changes

Workspace aggregation

Layer / File(s) Summary
Fallback anchor ID conversion
Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swift
Empty groups now convert the namespaced group ID raw value to MobileWorkspacePreview.ID before assigning anchorWorkspaceID.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 6fcde

This PR makes a localized ID conversion to restore iOS app-host unit-test compatibility, with no actionable merge-blocking risk remaining beyond normal checks and review.

Suggested reviewers: azooz2003-bit

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description clearly explains what changed and why, but it omits the required Testing section, checklist, review trigger, and an explicit Demo Video status. Add the required Testing section with test commands and verification results. Add the checklist and complete each applicable item. Include the review trigger block and state that a demo video is not applicable, or provide a video if needed.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the iOS ID conversion and the resulting app-host unit test fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
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.
Cmux Swift Actor Isolation ✅ Passed PASS: The pull request changes one assignment expression in production code. It converts stamped.id.rawValue into MobileWorkspacePreview.ID; it does not add or modify an actor, @MainActor, proto…
Cmux Swift Blocking Runtime ✅ Passed PASS: The PR changes one production Swift assignment in MobileWorkspaceAggregation.swift. It replaces a typed ID assignment with MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue). The diff …
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes only MobileWorkspaceAggregation.swift, replacing one ID conversion. The parent-to-HEAD diff contains no browser.* command, WebKit callback, socket-worker router, `pr…
Cmux Expensive Synchronous Load ✅ Passed PASS. The diff changes one assignment in MobileWorkspaceAggregation.derivedGroups: it constructs MobileWorkspacePreview.ID from stamped.id.rawValue. The changed file contains no agent-history lo…
Cmux Cache Substitution Correctness ✅ Passed PASS: The one-line diff only converts stamped.id.rawValue to MobileWorkspacePreview.ID. It does not replace an authoritative read with a cached or opportunistic value. MobileWorkspaceAggregation…
Cmux No Hacky Sleeps ✅ Passed PASS. The diff changes one line in a Swift source file. It converts stamped.id.rawValue to MobileWorkspacePreview.ID; it adds no sleep, timer, polling, delay, or wall-clock wait. The rule applies …
Cmux Algorithmic Complexity ✅ Passed PASS — The commit changes one line in MobileWorkspaceAggregation.swift. It replaces a typed-ID assignment with MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue), which is an O(1) conversion…
Cmux Swift Concurrency ✅ Passed PASS: The HEAD commit changes only one assignment in MobileWorkspaceAggregation.swift, replacing a typed-ID assignment with MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue). The diff adds …
Cmux Swift @Concurrent ✅ Passed The diff changes only the synchronous derivedGroups implementation. It converts stamped.id.rawValue into MobileWorkspacePreview.ID and introduces no async, nonisolated, @concurrent, actor-…
Cmux Swift Package Boundaries ✅ Passed PASS: The one-line production change is already inside the CmuxMobileShellModel SwiftPM target at Packages/iOS/CmuxMobileShellModel/Sources/.... Its Package.swift defines the library target and …
Cmux Swiftpm Lockfiles ✅ Passed PASS: The PR changes only one Swift source file in Packages/iOS/CmuxMobileShellModel. The parent-to-HEAD diff contains no Package.swift, Package.resolved, .gitignore, workflow, or Xcode projec…
Cmux Swift Logging ✅ Passed PASS: The pull request changes one assignment in MobileWorkspaceAggregation.swift, from stamped.id to MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue). The diff adds or changes no `print…
Cmux User-Facing Error Privacy ✅ Passed PASS. The diff changes only an identifier type conversion in MobileWorkspaceAggregation.swift. It adds no user-facing error, alert, command output, API error body, or recovery copy. The nearby text …
Cmux Full Internationalization ✅ Passed PASS. The exact diff changes one Swift assignment from stamped.id to MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue). It adds no user-facing text, localization key, string catalog or Info…
Cmux Swiftui State Layout ✅ Passed PASS. The PR changes one expression in MobileWorkspaceAggregation.swift, a Foundation/model aggregation file. The diff only converts stamped.id.rawValue into MobileWorkspacePreview.ID. It introd…
Cmux Architecture Rethink ✅ Passed PASS — The diff is a one-line type-correctness fix in MobileWorkspaceAggregation.derivedGroups. It converts the namespaced group ID through rawValue into the declared MobileWorkspacePreview.ID t…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The pull request changes only MobileWorkspaceAggregation.swift, a pure iOS workspace-model aggregation file. The one-line change converts stamped.id.rawValue to MobileWorkspacePreview.ID; …
Cmux Source Artifacts ✅ Passed PASS: The PR changes one tracked hand-written Swift source file, Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swift. The diff changes one assignment and …
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS. The pull request changes one production Swift line in Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swift. It replaces an ID type conversion with `M…
Cmux No Ambient Global State ✅ Passed PASS. The pull request changes one assignment in MobileWorkspaceAggregation.swift: it converts stamped.id.rawValue to MobileWorkspacePreview.ID. It adds no top-level function, mutable global, st…
Full details: Cmux Swift Actor Isolation

Explanation

PASS: The pull request changes one assignment expression in production code. It converts stamped.id.rawValue into MobileWorkspacePreview.ID; it does not add or modify an actor, @MainActor, protocol, reference type, model declaration, or store access. Both ID types and their initializers are pure Sendable value types, and MobileWorkspaceAggregation remains a Sendable struct. Therefore, the diff does not introduce or worsen any listed actor-isolation mistake.

Full details: Cmux Swift Blocking Runtime

Explanation

PASS: The PR changes one production Swift assignment in MobileWorkspaceAggregation.swift. It replaces a typed ID assignment with MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue). The diff introduces no semaphore, blocking wait, sleep, delayed dispatch, polling, main-queue sync, timer, or manual lock. The custom check therefore has no failure condition to report.

Full details: Cmux Browser Automation Off-Main

Explanation

PASS: The pull request changes only MobileWorkspaceAggregation.swift, replacing one ID conversion. The parent-to-HEAD diff contains no browser.* command, WebKit callback, socket-worker router, processV2Command, or policy-test change. The browser automation off-main failure conditions are not applicable.

Full details: Cmux Expensive Synchronous Load

Explanation

PASS. The diff changes one assignment in MobileWorkspaceAggregation.derivedGroups: it constructs MobileWorkspacePreview.ID from stamped.id.rawValue. The changed file contains no agent-history loader, file read, JSON/JSONL parsing, directory scan, or interactive-path handler. The change therefore does not add or move an expensive synchronous load onto the main actor or an interactive path.

Full details: Cmux Cache Substitution Correctness

Explanation

PASS: The one-line diff only converts stamped.id.rawValue to MobileWorkspacePreview.ID. It does not replace an authoritative read with a cached or opportunistic value. MobileWorkspaceAggregation is a pure derivation over MacWorkspaceState, and the changed branch creates a transient UI row identity for an empty group. No persistence, history, undo, snapshot, or cache path is changed.

Full details: Cmux No Hacky Sleeps

Explanation

PASS. The diff changes one line in a Swift source file. It converts stamped.id.rawValue to MobileWorkspacePreview.ID; it adds no sleep, timer, polling, delay, or wall-clock wait. The rule applies to non-Swift app/runtime changes, so this check has no applicable failure condition.

Full details: Cmux Algorithmic Complexity

Explanation

PASS — The commit changes one line in MobileWorkspaceAggregation.swift. It replaces a typed-ID assignment with MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue), which is an O(1) conversion. It does not add collection scans, sorting, filtering, joins, or batch rescans. The existing derivedGroups collection work is unchanged.

Full details: Cmux Swift Concurrency

Explanation

PASS: The HEAD commit changes only one assignment in MobileWorkspaceAggregation.swift, replacing a typed-ID assignment with MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue). The diff adds no DispatchQueue, DispatchGroup, Combine, completion-handler API, or fire-and-forget Task pattern. The target file contains no async/task constructs, so the Swift concurrency check has no applicable failure.

Full details: Cmux Swift `@Concurrent`

Explanation

The diff changes only the synchronous derivedGroups implementation. It converts stamped.id.rawValue into MobileWorkspacePreview.ID and introduces no async, nonisolated, @concurrent, actor-isolation, or heavy async call-site changes. The Swift concurrency check does not apply.

Full details: Cmux Swift Package Boundaries

Explanation

PASS: The one-line production change is already inside the CmuxMobileShellModel SwiftPM target at Packages/iOS/CmuxMobileShellModel/Sources/.... Its Package.swift defines the library target and a matching test target. The diff only changes ID construction and does not introduce or materially expand app-target domain logic.

Full details: Cmux Swiftpm Lockfiles

Explanation

PASS: The PR changes only one Swift source file in Packages/iOS/CmuxMobileShellModel. The parent-to-HEAD diff contains no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project changes. The package manifest is unchanged, so no SwiftPM lockfile update is required.

Full details: Cmux Swift Logging

Explanation

PASS: The pull request changes one assignment in MobileWorkspaceAggregation.swift, from stamped.id to MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue). The diff adds or changes no print, debugPrint, dump, NSLog, file logging, Logger, or sensitive-data logging. The change is outside the Swift logging failure conditions.

Full details: Cmux User-Facing Error Privacy

Explanation

PASS. The diff changes only an identifier type conversion in MobileWorkspaceAggregation.swift. It adds no user-facing error, alert, command output, API error body, or recovery copy. The nearby text is a developer-only code comment, which the rule allows.

Full details: Cmux Full Internationalization

Explanation

PASS. The exact diff changes one Swift assignment from stamped.id to MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue). It adds no user-facing text, localization key, string catalog or Info.plist entry, web content, metadata, API response, markdown, or changelog. Therefore no full-internationalization failure condition applies.

Full details: Cmux Swiftui State Layout

Explanation

PASS. The PR changes one expression in MobileWorkspaceAggregation.swift, a Foundation/model aggregation file. The diff only converts stamped.id.rawValue into MobileWorkspacePreview.ID. It introduces no SwiftUI import, observable state, geometry measurement, lazy/list row store reference, or render-time state mutation. The SwiftUI state/layout check is therefore not violated.

Full details: Cmux Architecture Rethink

Explanation

PASS — The diff is a one-line type-correctness fix in MobileWorkspaceAggregation.derivedGroups. It converts the namespaced group ID through rawValue into the declared MobileWorkspacePreview.ID type. The existing comments and model documentation define the invariant: empty groups use a stable UI identity, not a workspace capability. The change adds no timing, blocking, observer, side-channel, duplicate wiring, or UI lifecycle ownership. It matches the rule's allowed small local correctness fixes.

Full details: Cmux Swift Auxiliary Window Close Shortcuts

Explanation

PASS. The pull request changes only MobileWorkspaceAggregation.swift, a pure iOS workspace-model aggregation file. The one-line change converts stamped.id.rawValue to MobileWorkspacePreview.ID; it does not add or materially change an NSWindow, NSPanel, NSWindowController, SwiftUI Window, or WindowGroup. It also adds no window identifier, close-shortcut routing, or auxiliary-window registration. The auxiliary-window rule is therefore inapplicable, and the CI lint script does not apply to this diff.

Full details: Cmux Source Artifacts

Explanation

PASS: The PR changes one tracked hand-written Swift source file, Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swift. The diff changes one assignment and adds no logs, screenshots, recordings, caches, build output, temporary directories, dependency checkouts, or other artifact paths. This is intentional source code under the rule's pass criteria.

Full details: Cmux No Test Or Debug Seam In Production Source

Explanation

PASS. The pull request changes one production Swift line in Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swift. It replaces an ID type conversion with MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue). The diff adds no #if DEBUG or test-build guard, test/debug-named member, visibility widening, or test-only accessor. The changed code only resolves the stated ID assignment and does not introduce a test or debug seam.

Full details: Cmux No Ambient Global State

Explanation

PASS. The pull request changes one assignment in MobileWorkspaceAggregation.swift: it converts stamped.id.rawValue to MobileWorkspacePreview.ID. It adds no top-level function, mutable global, stub namespace type, or singleton. The change does not violate the ambient global state rule.

  • Fix all pre-merge checks with AI
✨ 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 fix/ios-group-anchor-id

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.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Closing as superseded: main already has stamped.anchorWorkspaceID = MobileWorkspacePreview.ID(rawValue: stamped.id.rawValue) via #8631.

The branch is kept; reopen if I've got this wrong. Part of the backlog cleanup in manaflow-ai/cmuxterm-hq#563.

@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 24, 2026
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.

2 participants