Repository navigation
Support JSONC Dock configs - #4263
Closed
mattpetters wants to merge 3 commits into
Closed
mattpetters wants to merge 3 commits into
mattpetters wants to merge 3 commits into
Conversation
|
@mattpetters is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDock configuration now accepts JSONC (comments and trailing commas). A new DockConfigParser preprocesses JSONC then decodes controls. Error localization, the DockEmptyView prompt, unit tests, documentation, and project build entries are updated accordingly. ChangesJSONC Dock Configuration Support
Sequence Diagram(s)sequenceDiagram
participant loadConfig
participant DockConfigParser
participant JSONCParser
participant JSONDecoder
participant Validation
loadConfig->>DockConfigParser: decodeControls(data)
DockConfigParser->>JSONCParser: preprocess(rawData)
JSONCParser-->>DockConfigParser: preprocessedJSON or throw
DockConfigParser->>JSONDecoder: decode DockConfigFile
JSONDecoder-->>DockConfigParser: DockConfigFile.controls
DockConfigParser-->>loadConfig: controls or NSError(cmux.dock)
loadConfig->>Validation: validate duplicate ids
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Author
|
BTW — love cmux. it’s my daily env now. you guys are tapped tf in! @lawrencecchen |
Contributor
Greptile SummaryThis PR introduces JSONC support for Dock configuration files, allowing users to write comments and trailing commas in their
Confidence Score: 5/5Safe to merge — changes are additive, well-tested, and correctly isolated. The parsing layer is cleanly separated into DockConfigParser, JSONC preprocessing errors are wrapped with user-friendly cmux-domain messages and recovery suggestions, and the file-locator logic is consistent across all three call sites. Previous review comments about error copy and doc precedence order have been addressed. Tests cover the new parsing paths, file-selection precedence, and the error message/recovery surface. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[DockControlsStore.reload] --> B[Self.resolve]
B --> C{Project config?}
C -->|walk up tree| D[DockConfigFileLocator.existingConfigURL]
D -->|dock.json exists?| E[dock.json URL]
D -->|dock.jsonc exists?| F[dock.jsonc URL]
D -->|neither| G[check parent dir]
G --> D
C -->|no project config| H[globalConfigURL]
H --> D2[DockConfigFileLocator.existingConfigURL in ~/.config/cmux]
E --> I[loadConfig]
F --> I
H --> I
I --> J[Data contentsOf url]
J --> K[DockConfigParser.decodeControls]
K --> L[JSONCParser.preprocess]
L -->|success| M[JSONDecoder decode DockConfigFile]
L -->|JSONC error| N[wrap to NSError with localised description and recovery]
M -->|success| O[DockControlRuntime array]
M -->|DecodingError| P[raw DecodingError propagates]
N --> P
P --> Q[errorMessage and errorRecoverySuggestion in DockErrorView]
O --> R[controls shown in UI]
Reviews (8): Last reviewed commit: "Merge branch 'manaflow-ai:main' into fea..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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
🤖 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 `@docs/dock.md`:
- Line 51: Update the Agent Setup sentence that currently says “validate the
JSON” so it instead reads “validate the JSONC” to match the earlier statement
that Dock config is JSONC; locate the Agent Setup wording in docs/dock.md (the
paragraph mentioning validation) and replace the phrase "validate the JSON" with
"validate the JSONC" so the terminology is consistent across the document.
In `@Sources/DockPanelView.swift`:
- Around line 86-105: The DockConfigParser enum (including static func
decodeControls) which references JSONCParser, DockControlDefinition, and
DockConfigFile should be extracted from Sources/DockPanelView.swift into its own
Swift source file (e.g., DockConfigParser.swift) and moved into the appropriate
module boundary so parsing logic is separated from UI/persistence code; create
the new file, paste the enum and its decodeControls implementation, add any
necessary imports or access-level adjustments (public/internal) so JSONCParser,
DockControlDefinition, and DockConfigFile remain visible, remove the original
enum from DockPanelView.swift, and run a quick build to fix any symbol
visibility or import issues.
- Around line 92-100: The NSError construction for domain "cmux.dock" with code
2 currently injects raw error.localizedDescription into the user-facing
NSLocalizedDescriptionKey; change this to a stable, user-friendly message (e.g.,
"Failed to preprocess JSONC. Please check your configuration and try again.")
and include only minimal/sanitized diagnostic info (for example append a short
diagnostic tag or error code, not the full upstream message) via a separate
developer-only field in userInfo (or log the full error). Update the NSError
creation where NSLocalizedDescriptionKey is set (the initializer creating the
userInfo dictionary) to use the new wording and move detailed diagnostics out of
the displayed string.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 1bf2a7cd-4cf9-4302-b648-dfa05c13ea45
📒 Files selected for processing (5)
Resources/Localizable.xcstringsSources/DockEmptyView.swiftSources/DockPanelView.swiftcmuxTests/CmuxConfigTests.swiftdocs/dock.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
mattpetters
force-pushed
the
feat/dock-should-support-jsonc
branch
from
May 16, 2026 22:08
d1b57e6 to
fd6a3f5
Compare
There was a problem hiding this comment.
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 `@Resources/Localizable.xcstrings`:
- Around line 108121-108134: Update the localized message for the
"dock.error.jsoncParseFailed" stringUnit so it follows the preprocessing format
contract by including the underlying error placeholder ("%@") in the value;
modify both localizations (the "en" and "ja" stringUnit values) to include the
"%@" placeholder where the underlying error text should be interpolated so
downstream preprocessing can surface the detailed error.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 815b1038-b9a2-4236-a814-a8831b18d86d
📒 Files selected for processing (7)
Resources/Localizable.xcstringsSources/DockConfigParser.swiftSources/DockEmptyView.swiftSources/DockPanelView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CmuxConfigTests.swiftdocs/dock.md
mattpetters
force-pushed
the
feat/dock-should-support-jsonc
branch
from
May 16, 2026 22:24
fd6a3f5 to
7eef9e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@cmuxTests/CmuxConfigTests.swift`:
- Around line 38-60: Add a new unit test (e.g.,
testPreservesCommentMarkersInStringValues) that uses decodeControls to parse a
JSON controls array where a control's "command" string contains explicit comment
markers like "//" and "/* ... */"; assert controls.count == 1 and that
controls.first?.command exactly equals the original string so comment markers
inside string values are preserved; reference the existing
testParsesBlockCommentedControlAndPreservesCommentSyntaxInStrings for structure
and reuse decodeControls, controls, and XCTAssertEqual assertions.
In `@Resources/Localizable.xcstrings`:
- Around line 108155-108171: Update the English localization entries for the
JSONC parse failure reasons to standardize article usage: change the value for
"dock.error.jsoncParseFailed.reason.encoding" from "unsupported text encoding"
to "an unsupported text encoding", and change the value for
"dock.error.jsoncParseFailed.reason.syntax" to a more natural phrase such as
"invalid syntax" (or "an invalid JSONC syntax" if you prefer to keep an
article), so composed messages like "Couldn't parse Dock config as JSONC: %@."
read consistently and naturally.
In `@Sources/DockConfigParser.swift`:
- Around line 37-40: Update the recovery suggestion string in DockConfigParser
where NSLocalizedRecoverySuggestionErrorKey is set for the
"dock.error.jsoncParseFailed.recovery" message: change the defaultValue (and
matching localized copy if present) to reference both "dock.json" and
"dock.jsonc" (e.g. "Check comments and trailing commas in dock.json or
dock.jsonc, then reload Dock.") so users editing either format see the correct
guidance.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 09089e2c-29bc-4add-8e33-2ae21fffc2ac
📒 Files selected for processing (7)
Resources/Localizable.xcstringsSources/DockConfigParser.swiftSources/DockEmptyView.swiftSources/DockPanelView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CmuxConfigTests.swiftdocs/dock.md
mattpetters
force-pushed
the
feat/dock-should-support-jsonc
branch
from
May 16, 2026 22:31
7eef9e0 to
b8cd040
Compare
There was a problem hiding this comment.
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 `@Resources/Localizable.xcstrings`:
- Around line 107906-107913: The deliverable text currently mandates
creating/updating only "dock.json" which conflicts with JSONC support; update
the "Deliverable" section so it accepts either "dock.json" or "dock.jsonc"
(e.g., change "Create or update the appropriate dock.json." to "Create or update
the appropriate dock.json or dock.jsonc."), preserve the existing guidance
around precedence ("if both exist, dock.json wins") and JSONC parsing
validation, and ensure references to the top-level "controls" schema remain
consistent with both filenames.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 8a58662a-b89d-49cd-9bea-5739348862e3
📒 Files selected for processing (7)
Resources/Localizable.xcstringsSources/DockConfigParser.swiftSources/DockEmptyView.swiftSources/DockPanelView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CmuxConfigTests.swiftdocs/dock.md
mattpetters
force-pushed
the
feat/dock-should-support-jsonc
branch
from
May 16, 2026 22:38
b8cd040 to
d8c0026
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/DockPanelView.swift (2)
300-304:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve the parser’s recovery guidance in the Dock error UI.
DockConfigParser.decodeControlsnow returns user-facing recovery steps, but this layer still collapses failures toerror.localizedDescriptionandDockErrorViewonly renders a single string. The new JSONC “what to do next” guidance never reaches users. Please carrylocalizedRecoverySuggestionthrough the store and render it alongside the message.As per coding guidelines: “ensure messages: (1) describe the issue in cmux/product terms, (2) provide 1–2 concrete next actions”.
Also applies to: 418-419, 627-628, 850-867
🤖 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 `@Sources/DockPanelView.swift` around lines 300 - 304, The catch block in DockPanelView currently sets only errorMessage = error.localizedDescription, dropping DockConfigParser.decodeControls’ localizedRecoverySuggestion; update the error flow to carry a separate recoverySuggestion (or a combined structured error object) from where decodeControls returns it through the store/update methods, set both sourceLabel and recoverySuggestion in the catch (instead of collapsing to a single string), and update DockErrorView to render the recoverySuggestion as a second, actionable line (1–2 concrete next actions) alongside the existing error message; ensure the same propagation/fix is applied to the other catch sites referenced (around lines 418–419 and 627–628 and 850–867).
320-331:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOpen the existing JSONC file when reload fails.
If the parser throws,
activeConfigURLnever gets populated, so the Line 326 fallback still targetsdock.json. For an existing but malformeddock.jsonc, the “Open Dock Config” action will open or create a newdock.json, and becauseDockConfigFileLocatorprefers.json, that new file can shadow the real config on the next reload instead of letting the user fix it. Please prefer any existing config file before falling back to the default path.Suggested fix
private static func preferredEditableConfigURL(rootDirectory: String?) throws -> URL { if let rootDirectory = rootDirectory.flatMap(existingDirectory) { - return URL(fileURLWithPath: rootDirectory, isDirectory: true) - .appendingPathComponent(".cmux", isDirectory: true) - .appendingPathComponent("dock.json", isDirectory: false) + let configDirectory = URL(fileURLWithPath: rootDirectory, isDirectory: true) + .appendingPathComponent(".cmux", isDirectory: true) + return DockConfigFileLocator.existingConfigURL(in: configDirectory) + ?? configDirectory.appendingPathComponent("dock.json", isDirectory: false) } - return defaultGlobalConfigURL() + return globalConfigURL() ?? defaultGlobalConfigURL() }Also applies to: 456-458, 473-503
🤖 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 `@Sources/DockPanelView.swift` around lines 320 - 331, openConfiguration currently falls back to Self.preferredEditableConfigURL(rootDirectory: lastRootDirectory) even when a malformed but existing config (e.g. dock.jsonc) exists because activeConfigURL is nil after a parse error; change the logic in openConfiguration (and the similar blocks at the other locations mentioned) to first check for any existing config file in the root directory before using preferredEditableConfigURL: attempt to locate an existing config URL (using the same lookup logic as DockConfigFileLocator or an existing helper that returns an existing config URL for lastRootDirectory), set target to that existing URL if present, otherwise use preferredEditableConfigURL, and then proceed to create the template only if the target truly does not exist (preserving activeConfigURL behavior and avoiding creating a new dock.json that would shadow dock.jsonc).
🤖 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 `@docs/dock.md`:
- Around line 62-67: Update the trust section to explicitly state that the
global Dock config trust behavior applies to both ~/.config/cmux/dock.json and
~/.config/cmux/dock.jsonc (i.e., both filenames are treated the same for trust
decisions); mention that either file in the global config location will be
subject to the same trust rules used for repo-level .cmux/dock.json and
.cmux/dock.jsonc so readers know there is no trust difference between the .json
and .jsonc variants.
---
Outside diff comments:
In `@Sources/DockPanelView.swift`:
- Around line 300-304: The catch block in DockPanelView currently sets only
errorMessage = error.localizedDescription, dropping
DockConfigParser.decodeControls’ localizedRecoverySuggestion; update the error
flow to carry a separate recoverySuggestion (or a combined structured error
object) from where decodeControls returns it through the store/update methods,
set both sourceLabel and recoverySuggestion in the catch (instead of collapsing
to a single string), and update DockErrorView to render the recoverySuggestion
as a second, actionable line (1–2 concrete next actions) alongside the existing
error message; ensure the same propagation/fix is applied to the other catch
sites referenced (around lines 418–419 and 627–628 and 850–867).
- Around line 320-331: openConfiguration currently falls back to
Self.preferredEditableConfigURL(rootDirectory: lastRootDirectory) even when a
malformed but existing config (e.g. dock.jsonc) exists because activeConfigURL
is nil after a parse error; change the logic in openConfiguration (and the
similar blocks at the other locations mentioned) to first check for any existing
config file in the root directory before using preferredEditableConfigURL:
attempt to locate an existing config URL (using the same lookup logic as
DockConfigFileLocator or an existing helper that returns an existing config URL
for lastRootDirectory), set target to that existing URL if present, otherwise
use preferredEditableConfigURL, and then proceed to create the template only if
the target truly does not exist (preserving activeConfigURL behavior and
avoiding creating a new dock.json that would shadow dock.jsonc).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: fa8b6ee9-ae0d-479d-be1c-86699caf50cb
📒 Files selected for processing (7)
Resources/Localizable.xcstringsSources/DockConfigParser.swiftSources/DockEmptyView.swiftSources/DockPanelView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CmuxConfigTests.swiftdocs/dock.md
mattpetters
force-pushed
the
feat/dock-should-support-jsonc
branch
from
May 16, 2026 22:46
d8c0026 to
9fe3fb3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DockConfigParsersource file and surface stable, user-facing parse errors with recovery guidance.Validation
git diff --checkpython3 -m json.tool Resources/Localizable.xcstringsCMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag dock-jsoncTests were not run locally per repository policy; the new behavior is covered by
DockConfigParserTestsfor CI.Summary by CodeRabbit
New Features
Documentation
Bug Fixes / UX
Tests