feat(macos): add OpenWorlds product shell - #123
Conversation
📝 WalkthroughWalkthroughThis PR introduces the OpenWorlds design system—a complete theme library with color palette, reusable SwiftUI components, and integrated campaign launcher—alongside AppProcessService cleanup and design governance documentation. The implementation flows from foundational theme constants through composed UI components into updated root and play views. ChangesOpenWorlds UI Design System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
macos/ClawDnDApp/Sources/ClawDnDApp/Services/AppProcessService.swift (1)
252-258:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClear activeCampaignID in the viewer termination handler for consistency.
The explicit
stopViewer()call now clearsactiveCampaignID(line 106), but when the viewer process exits naturally or crashes, the termination handler updatesviewerEndpoint.statuswithout clearingactiveCampaignID. This creates an inconsistency where the UI may show a stale active campaign after an unexpected viewer exit.🔄 Proposed fix to maintain state consistency
} else if self?.viewerProcess === managed { self?.viewerProcess = nil + self?.activeCampaignID = nil if var endpoint = self?.viewerEndpoint { endpoint.status = .stopped self?.viewerEndpoint = endpoint } }🤖 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 `@macos/ClawDnDApp/Sources/ClawDnDApp/Services/AppProcessService.swift` around lines 252 - 258, When handling the viewer process termination in the block that checks `self?.viewerProcess === managed`, also clear the stale campaign state: set `self?.activeCampaignID = nil` (or call the existing `stopViewer()` logic) alongside setting `self?.viewerProcess = nil` and updating `viewerEndpoint.status = .stopped`, ensuring `activeCampaignID` is cleared the same way as when `stopViewer()` is invoked.
🤖 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 `@macos/ClawDnDApp/Sources/ClawDnDApp/Views/PlayView.swift`:
- Around line 34-40: Remove the redundant call to selectFirstCampaignIfNeeded
from the View's .onAppear block because campaignStore.reload (invoked there) is
asynchronous and the onChange(of: campaignStore.campaigns) already handles
selecting when campaigns finish loading; edit the PlayView.swift onAppear
closure to only call campaignStore.reload(repoPath: repoPath) and leave
selectFirstCampaignIfNeeded only in the onChange handler so selection happens
after campaigns update.
- Around line 238-240: Change the computed property selectedCampaign to return
nil when there is no matching campaign instead of falling back to
campaigns.first; specifically remove the "?? campaigns.first" fallback in the
selectedCampaign getter (which currently compares $0.id to selectedCampaignID),
rely on selectFirstCampaignIfNeeded to ensure consistent selection, and update
campaignPreview to handle a nil selectedCampaign explicitly (e.g., show "No
campaign selected") so stale selectedCampaignID cases aren't silently masked.
In `@macos/ClawDnDApp/Sources/ClawDnDApp/Views/RootView.swift`:
- Around line 49-52: The currentCampaign computed property falls back to
campaignStore.campaigns.first when processService.activeCampaignID is set but
missing, which can surface stale/incorrect campaign info; change currentCampaign
to only return a match for processService.activeCampaignID
(campaignStore.campaigns.first { $0.id == processService.activeCampaignID }) and
otherwise return nil so the title shows "No Chronicle Selected" instead of a
fallback campaign.
---
Outside diff comments:
In `@macos/ClawDnDApp/Sources/ClawDnDApp/Services/AppProcessService.swift`:
- Around line 252-258: When handling the viewer process termination in the block
that checks `self?.viewerProcess === managed`, also clear the stale campaign
state: set `self?.activeCampaignID = nil` (or call the existing `stopViewer()`
logic) alongside setting `self?.viewerProcess = nil` and updating
`viewerEndpoint.status = .stopped`, ensuring `activeCampaignID` is cleared the
same way as when `stopViewer()` is invoked.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1c7a903b-4fe1-4f48-b819-a499ee7a7e1a
📒 Files selected for processing (6)
docs/OPENWORLDS_DESIGN_ASSET_POLICY.mdmacos/ClawDnDApp/Sources/ClawDnDApp/Services/AppProcessService.swiftmacos/ClawDnDApp/Sources/ClawDnDApp/Views/Design/OpenWorldsComponents.swiftmacos/ClawDnDApp/Sources/ClawDnDApp/Views/Design/OpenWorldsTheme.swiftmacos/ClawDnDApp/Sources/ClawDnDApp/Views/PlayView.swiftmacos/ClawDnDApp/Sources/ClawDnDApp/Views/RootView.swift
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (2)
macos/ClawDnDApp/Sources/ClawDnDApp/Views/RootView.swift (1)
macos/ClawDnDApp/Sources/ClawDnDApp/Views/Design/OpenWorldsComponents.swift (1)
background(193-203)
macos/ClawDnDApp/Sources/ClawDnDApp/Views/PlayView.swift (2)
macos/ClawDnDApp/Sources/ClawDnDApp/Services/AppProcessService.swift (4)
startViewer(53-97)stopProvider(192-205)stopViewer(99-107)startProviderSession(109-190)macos/ClawDnDApp/Sources/ClawDnDApp/Services/CampaignStore.swift (1)
reload(10-31)
🪛 LanguageTool
docs/OPENWORLDS_DESIGN_ASSET_POLICY.md
[uncategorized] ~22-~22: The operating system from Apple is written “macOS”.
Context: ... visual/design implementation scoped to macos/, viewer/, docs/, or build scripts...
(MAC_OS)
🔇 Additional comments (17)
docs/OPENWORLDS_DESIGN_ASSET_POLICY.md (1)
1-25: LGTM!macos/ClawDnDApp/Sources/ClawDnDApp/Views/Design/OpenWorldsTheme.swift (1)
1-94: LGTM!macos/ClawDnDApp/Sources/ClawDnDApp/Views/Design/OpenWorldsComponents.swift (1)
1-205: LGTM!macos/ClawDnDApp/Sources/ClawDnDApp/Services/AppProcessService.swift (1)
106-106: LGTM!macos/ClawDnDApp/Sources/ClawDnDApp/Views/RootView.swift (4)
23-47: LGTM!
118-165: LGTM!
167-219: LGTM!
221-260: LGTM!macos/ClawDnDApp/Sources/ClawDnDApp/Views/PlayView.swift (9)
3-23: LGTM!
48-88: LGTM!
90-108: LGTM!
110-136: LGTM!
138-150: LGTM!
165-185: LGTM!
204-209: LGTM!
279-413: LGTM!
415-464: LGTM!
| .onAppear { | ||
| campaignStore.reload(repoPath: repoPath) | ||
| selectFirstCampaignIfNeeded() | ||
| } | ||
| .onChange(of: campaignStore.campaigns) { _ in | ||
| selectFirstCampaignIfNeeded() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Remove redundant selectFirstCampaignIfNeeded call in onAppear.
campaignStore.reload is async (uses Task.detached), so the selectFirstCampaignIfNeeded call on Line 36 runs before campaigns are loaded. The onChange handler (Lines 38-40) already ensures selection when campaigns update, making the onAppear call redundant.
♻️ Proposed fix
.onAppear {
campaignStore.reload(repoPath: repoPath)
- selectFirstCampaignIfNeeded()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .onAppear { | |
| campaignStore.reload(repoPath: repoPath) | |
| selectFirstCampaignIfNeeded() | |
| } | |
| .onChange(of: campaignStore.campaigns) { _ in | |
| selectFirstCampaignIfNeeded() | |
| } | |
| .onAppear { | |
| campaignStore.reload(repoPath: repoPath) | |
| } | |
| .onChange(of: campaignStore.campaigns) { _ in | |
| selectFirstCampaignIfNeeded() | |
| } |
🤖 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 `@macos/ClawDnDApp/Sources/ClawDnDApp/Views/PlayView.swift` around lines 34 -
40, Remove the redundant call to selectFirstCampaignIfNeeded from the View's
.onAppear block because campaignStore.reload (invoked there) is asynchronous and
the onChange(of: campaignStore.campaigns) already handles selecting when
campaigns finish loading; edit the PlayView.swift onAppear closure to only call
campaignStore.reload(repoPath: repoPath) and leave selectFirstCampaignIfNeeded
only in the onChange handler so selection happens after campaigns update.
| private var selectedCampaign: CampaignSummary? { | ||
| campaigns.first { $0.id == selectedCampaignID } ?? campaigns.first | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider removing fallback to .first for clarity.
The fallback ?? campaigns.first could mask issues where selectedCampaignID is stale. Since selectFirstCampaignIfNeeded maintains selection consistency, this fallback should rarely trigger. Returning nil explicitly would make the no-selection case clearer in the UI (e.g., campaignPreview shows "No campaign selected").
♻️ Proposed change
private var selectedCampaign: CampaignSummary? {
- campaigns.first { $0.id == selectedCampaignID } ?? campaigns.first
+ campaigns.first { $0.id == selectedCampaignID }
}Then update campaignPreview to handle nil more explicitly if needed.
🤖 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 `@macos/ClawDnDApp/Sources/ClawDnDApp/Views/PlayView.swift` around lines 238 -
240, Change the computed property selectedCampaign to return nil when there is
no matching campaign instead of falling back to campaigns.first; specifically
remove the "?? campaigns.first" fallback in the selectedCampaign getter (which
currently compares $0.id to selectedCampaignID), rely on
selectFirstCampaignIfNeeded to ensure consistent selection, and update
campaignPreview to handle a nil selectedCampaign explicitly (e.g., show "No
campaign selected") so stale selectedCampaignID cases aren't silently masked.
| private var currentCampaign: CampaignSummary? { | ||
| campaignStore.campaigns.first { $0.id == processService.activeCampaignID } | ||
| ?? campaignStore.campaigns.first | ||
| } |
There was a problem hiding this comment.
Fallback to .first may display stale campaign data.
When activeCampaignID is set but doesn't match any loaded campaign, falling back to campaigns.first could show incorrect campaign info in the title bar. Better to return nil so the title bar shows "No Chronicle Selected".
🔧 Proposed fix
private var currentCampaign: CampaignSummary? {
- campaignStore.campaigns.first { $0.id == processService.activeCampaignID }
- ?? campaignStore.campaigns.first
+ guard let activeCampaignID = processService.activeCampaignID else {
+ return nil
+ }
+ return campaignStore.campaigns.first { $0.id == activeCampaignID }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private var currentCampaign: CampaignSummary? { | |
| campaignStore.campaigns.first { $0.id == processService.activeCampaignID } | |
| ?? campaignStore.campaigns.first | |
| } | |
| private var currentCampaign: CampaignSummary? { | |
| guard let activeCampaignID = processService.activeCampaignID else { | |
| return nil | |
| } | |
| return campaignStore.campaigns.first { $0.id == activeCampaignID } | |
| } |
🤖 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 `@macos/ClawDnDApp/Sources/ClawDnDApp/Views/RootView.swift` around lines 49 -
52, The currentCampaign computed property falls back to
campaignStore.campaigns.first when processService.activeCampaignID is set but
missing, which can surface stale/incorrect campaign info; change currentCampaign
to only return a match for processService.activeCampaignID
(campaignStore.campaigns.first { $0.id == processService.activeCampaignID }) and
otherwise return nil so the title shows "No Chronicle Selected" instead of a
fallback campaign.
|
Parking this PR as draft after visual review. The SwiftUI approximation does not preserve the OpenWorlds export closely enough to be a merge candidate. Next step is a fidelity-first planning sprint: treat the exported OpenWorlds HTML/JSX/CSS as the visual contract, decide the production integration architecture, then replace or substantially rework this PR from that plan. No merge until the new plan has screenshot-level validation. |
|
Supersession note: PR #124 is now open with the OpenWorlds fidelity-first contract. This draft PR should remain unmerged; its SwiftUI repaint direction is superseded by the audited viewer-hosted OpenWorlds surface path. Per the new plan, this PR can be closed as superseded once the first viewer-hosted |
|
Superseded by the OpenWorlds native-app correction path. The visible app shell should be the viewer-hosted OpenWorlds UI, with Swift/AppKit acting as process supervisor and native bridge. The corrected roadmap is now reflected in #82, #113, #114, #131, #132, and #133; this draft should not merge as the product UI direction. |
|
Superseded by #135. The correction path makes OpenWorlds the visible native app shell and keeps SwiftUI/AppKit as the supervisor/bridge layer instead of repainting the UI in the old Swift shell. |
1 participant
Summary
Refs #113, #114, #122.
This PR starts translating the OpenWorlds design bundle into ClawDnD's native macOS app without importing third-party/reference assets or changing engine state semantics.
activeCampaignIDresolves; stopping the viewer clears the active campaign marker.docs/OPENWORLDS_DESIGN_ASSET_POLICY.mdwith explicit audited source buckets forstyles.css,*.jsx,data.js, screenshots, uploaded references, and handoff metadata.Architecture Notes
The macOS app remains an orchestrator/read surface. This PR does not add engine writes, rules changes, provider protocol changes, story/QA content, world seeds, or asset ingestion.
State boundaries:
Resume Chroniclestill callsAppProcessService.startViewer(...)withstateDirandcampaignID.Start Gamestill callsAppProcessService.startProviderSession(...).play-state,qa/state, snapshots, or provider runtime files directly.uploads/*.pngas reference-only/do-not-commit.Design Source
Local scratch bundle:
/Volumes/LEXAR/Codex/openworlds-design-2026-05-25//Volumes/LEXAR/Codex/openworlds-design-2026-05-25/clawdnd-mac-app-gap-map.md/Volumes/LEXAR/Codex/openworlds-design-2026-05-25/clawdnd-openworlds-shell-smoke-2.pngNo third-party/reference image assets were copied into this PR.
Files To Review First
macos/ClawDnDApp/Sources/ClawDnDApp/Views/Design/OpenWorldsTheme.swiftmacos/ClawDnDApp/Sources/ClawDnDApp/Views/Design/OpenWorldsComponents.swiftmacos/ClawDnDApp/Sources/ClawDnDApp/Views/RootView.swiftmacos/ClawDnDApp/Sources/ClawDnDApp/Views/PlayView.swiftdocs/OPENWORLDS_DESIGN_ASSET_POLICY.mdValidation
Ran locally from
/Volumes/LEXAR/repos/ClawDnD-openworlds-product-shell:Also ran a visual smoke capture after
--verify:/Volumes/LEXAR/Codex/openworlds-design-2026-05-25/clawdnd-openworlds-shell-smoke-2.pngAdversarial Review
Two read-only subagents reviewed the uncommitted diff before publication.
Fixed before this PR:
ViewThatFitsand compacted column widths.activeCampaignIDon viewer stop.git diff --checkEOF whitespace warnings: fixed.Clearances from the reviews:
play-state/qa/state/ snapshot writes from visual code.Summary by CodeRabbit
New Features
Documentation