Repository navigation
Add customization and diagnostics skills - #4222
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughPR adds two new skills— ChangesSkills Catalog Refresh
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 15 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dc95e7e. Configure here.
Greptile SummaryAdds two new end-user skills (
Confidence Score: 5/5Safe to merge — changes are limited to new skill docs, a read-only diagnostics shell script, and docs/i18n updates; no runtime app logic is touched. The diagnostics script is purely additive and read-only: it only runs command -v, cmux ping, file-existence checks, and grep for markers. The $HOME redaction via safe_path is applied consistently, the settings_helper fallback chain correctly checks the Vercel install path before the Codex path, and set -euo pipefail is paired with || true/2>/dev/null guards everywhere a command can legitimately fail. The customization skill is documentation only. The docs page and i18n keys are consistent across the component and both language files. No files require special attention. Important Files Changed
Reviews (4): Last reviewed commit: "Guard diagnostics against special files" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if [[ ! -x "$settings_helper" && -x "$HOME/.codex/skills/cmux-settings/scripts/cmux-settings" ]]; then | ||
| settings_helper="$HOME/.codex/skills/cmux-settings/scripts/cmux-settings" | ||
| fi |
There was a problem hiding this comment.
~/.codex/skills/ but not ~/.agents/skills/, the install destination used by npx skills add. If a user installs cmux-diagnostics via skills.sh (writing to ~/.codex/skills/) while cmux-settings was installed via npx skills add (writing to ~/.agents/skills/), the relative-path resolution gives ~/.codex/skills/cmux-settings/… (not found), the fallback checks the same path again (no-op), and the report incorrectly prints cmux_settings_helper: missing even though the helper is present at ~/.agents/skills/.
| if [[ ! -x "$settings_helper" && -x "$HOME/.codex/skills/cmux-settings/scripts/cmux-settings" ]]; then | |
| settings_helper="$HOME/.codex/skills/cmux-settings/scripts/cmux-settings" | |
| fi | |
| if [[ ! -x "$settings_helper" && -x "$HOME/.agents/skills/cmux-settings/scripts/cmux-settings" ]]; then | |
| settings_helper="$HOME/.agents/skills/cmux-settings/scripts/cmux-settings" | |
| elif [[ ! -x "$settings_helper" && -x "$HOME/.codex/skills/cmux-settings/scripts/cmux-settings" ]]; then | |
| settings_helper="$HOME/.codex/skills/cmux-settings/scripts/cmux-settings" | |
| fi |
There was a problem hiding this comment.
Fixed by checking the Vercel install path before the Codex-only install path.
— Claude Code
| <CodeBlock title={t("installWithVercel")} lang="bash">{`npx skills add manaflow-ai/cmux --all -g -y | ||
| npx skills add manaflow-ai/cmux --skill cmux-diagnostics -g -y`}</CodeBlock> |
There was a problem hiding this comment.
cmux-diagnostics on top. Adding inline comments makes the intent unambiguous.
| <CodeBlock title={t("installWithVercel")} lang="bash">{`npx skills add manaflow-ai/cmux --all -g -y | |
| npx skills add manaflow-ai/cmux --skill cmux-diagnostics -g -y`}</CodeBlock> | |
| <CodeBlock title={t("installWithVercel")} lang="bash">{`# Install all cmux skills | |
| npx skills add manaflow-ai/cmux --all -g -y | |
| # Or install just the diagnostics skill | |
| npx skills add manaflow-ai/cmux --skill cmux-diagnostics -g -y`}</CodeBlock> |
There was a problem hiding this comment.
Fixed by labeling the Vercel install commands as alternatives in the docs code block.
— Claude Code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@skills/cmux-customization/SKILL.md`:
- Around line 21-38: The numbered workflow in SKILL.md has fenced code blocks
(```bash ... ```) that lack surrounding blank lines, triggering markdownlint
MD031; update the list items (notably the code blocks under steps 1, 3, and 5)
to insert a blank line before each opening ```bash and a blank line after each
closing ``` so each fenced block is separated from adjacent list text; keep the
code fence markers (```bash) and the surrounding numbered items intact while
only adding the required empty lines to satisfy markdownlint.
In `@skills/cmux-diagnostics/SKILL.md`:
- Around line 33-67: In SKILL.md, the fenced code blocks inside the numbered
checklist items (e.g., the blocks under "CLI and socket health:", "Settings
health:", "Hook installation:", "Session restore evidence:", and "Notification
path:") lack surrounding blank lines and trigger markdownlint MD031; edit the
file to insert a blank line immediately before each ```...``` fence and a blank
line immediately after each closing ``` so each fenced block is separated from
the surrounding list text (apply the same change for the three-line blocks under
"CLI and socket health" and "Settings health" and for every other fenced block
in that numbered section).
🪄 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: 8783f8e6-4dcf-4011-b665-3e8b6f1b2ce3
📒 Files selected for processing (11)
skills/cmux-customization/SKILL.mdskills/cmux-customization/agents/openai.yamlskills/cmux-debug-windows/SKILL.mdskills/cmux-debug-windows/agents/openai.yamlskills/cmux-debug-windows/scripts/debug_windows_snapshot.shskills/cmux-diagnostics/SKILL.mdskills/cmux-diagnostics/agents/openai.yamlskills/cmux-diagnostics/scripts/cmux-diagnosticsweb/app/[locale]/docs/skills/page.tsxweb/messages/en.jsonweb/messages/ja.json
💤 Files with no reviewable changes (3)
- skills/cmux-debug-windows/SKILL.md
- skills/cmux-debug-windows/agents/openai.yaml
- skills/cmux-debug-windows/scripts/debug_windows_snapshot.sh
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/cmux-diagnostics/scripts/cmux-diagnostics">
<violation number="1" location="skills/cmux-diagnostics/scripts/cmux-diagnostics:83">
P2: Add a fallback to `~/.agents/skills/cmux-settings/scripts/cmux-settings` before the Codex fallback so diagnostics can find the settings helper when skills were installed via `npx skills add`.</violation>
<violation number="2" location="skills/cmux-diagnostics/scripts/cmux-diagnostics:98">
P3: Avoid using the string sentinel `unset` to detect missing socket env vars; check variable presence instead so a real value of `unset` is not misclassified.
(Based on your team's feedback about checking key presence instead of string sentinels.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
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 `@skills/cmux-diagnostics/scripts/cmux-diagnostics`:
- Around line 53-77: The checks in file_summary and marker_summary use [[ -e
"$path" ]], which treats FIFOs/devices/sockets as "present" and can cause
wc/grep to block; update both functions to guard for regular files instead (use
[[ -f "$path" ]] or an equivalent test) before invoking wc, stat or grep, and
treat non-regular paths as missing (or emit a clear non-regular status) so
diagnostics won't block on special file types; locate the checks in file_summary
and marker_summary and replace the -e guard with a regular-file test.
🪄 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: adf42d3f-0b2f-4d8b-a63c-6bacafa3bac6
📒 Files selected for processing (1)
skills/cmux-diagnostics/scripts/cmux-diagnostics
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 `@skills/cmux-diagnostics/scripts/cmux-diagnostics`:
- Around line 104-109: The conditionals that check CMUX_SOCKET_PATH and
CMUX_SOCKET should use non-empty tests so empty values don't mask fallbacks:
replace the presence checks using "${CMUX_SOCKET_PATH+x}" and "${CMUX_SOCKET+x}"
with non-empty checks like -n "${CMUX_SOCKET_PATH:-}" and -n "${CMUX_SOCKET:-}"
in the conditional block that prints cmux_socket_path (which uses safe_path to
format the value), so an empty CMUX_SOCKET_PATH will no longer prevent falling
back to CMUX_SOCKET.
🪄 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: 6190a65a-60dd-4406-9d66-82092d01f2d6
📒 Files selected for processing (1)
skills/cmux-diagnostics/scripts/cmux-diagnostics
| if [[ "${CMUX_SOCKET_PATH+x}" == "x" ]]; then | ||
| printf 'cmux_socket_path: %s (from CMUX_SOCKET_PATH)\n' "$(safe_path "$CMUX_SOCKET_PATH")" | ||
| elif [[ "${CMUX_SOCKET+x}" == "x" ]]; then | ||
| printf 'cmux_socket_path: %s (from CMUX_SOCKET)\n' "$(safe_path "$CMUX_SOCKET")" | ||
| else | ||
| printf 'cmux_socket_path: unset\n' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show the current branch logic in the file.
sed -n '102,110p' skills/cmux-diagnostics/scripts/cmux-diagnostics
# 2) Demonstrate behavior difference when CMUX_SOCKET_PATH is set-but-empty.
CMUX_SOCKET_PATH="" CMUX_SOCKET="/tmp/cmux.sock" bash -lc '
if [[ "${CMUX_SOCKET_PATH+x}" == "x" ]]; then
echo "current: chose CMUX_SOCKET_PATH (value=\"${CMUX_SOCKET_PATH}\")"
elif [[ "${CMUX_SOCKET+x}" == "x" ]]; then
echo "current: chose CMUX_SOCKET=${CMUX_SOCKET}"
else
echo "current: unset"
fi
if [[ -n "${CMUX_SOCKET_PATH:-}" ]]; then
echo "proposed: chose CMUX_SOCKET_PATH=${CMUX_SOCKET_PATH}"
elif [[ -n "${CMUX_SOCKET:-}" ]]; then
echo "proposed: chose CMUX_SOCKET=${CMUX_SOCKET}"
else
echo "proposed: unset"
fi
'Repository: manaflow-ai/cmux
Length of output: 775
🏁 Script executed:
rg '\$\{[A-Z_]+\+x\}' skills/cmux-diagnostics/scripts/cmux-diagnosticsRepository: manaflow-ai/cmux
Length of output: 149
Use non-empty checks for socket env precedence.
At lines 104–106, ${VAR+x} treats an empty variable as "present," so an empty CMUX_SOCKET_PATH masks a valid CMUX_SOCKET and prints a blank path. Switch to -n "${VAR:-}" to skip empty values and fall back correctly.
Proposed fix
-if [[ "${CMUX_SOCKET_PATH+x}" == "x" ]]; then
+if [[ -n "${CMUX_SOCKET_PATH:-}" ]]; then
printf 'cmux_socket_path: %s (from CMUX_SOCKET_PATH)\n' "$(safe_path "$CMUX_SOCKET_PATH")"
-elif [[ "${CMUX_SOCKET+x}" == "x" ]]; then
+elif [[ -n "${CMUX_SOCKET:-}" ]]; then
printf 'cmux_socket_path: %s (from CMUX_SOCKET)\n' "$(safe_path "$CMUX_SOCKET")"
else
printf 'cmux_socket_path: unset\n'
fi📝 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.
| if [[ "${CMUX_SOCKET_PATH+x}" == "x" ]]; then | |
| printf 'cmux_socket_path: %s (from CMUX_SOCKET_PATH)\n' "$(safe_path "$CMUX_SOCKET_PATH")" | |
| elif [[ "${CMUX_SOCKET+x}" == "x" ]]; then | |
| printf 'cmux_socket_path: %s (from CMUX_SOCKET)\n' "$(safe_path "$CMUX_SOCKET")" | |
| else | |
| printf 'cmux_socket_path: unset\n' | |
| if [[ -n "${CMUX_SOCKET_PATH:-}" ]]; then | |
| printf 'cmux_socket_path: %s (from CMUX_SOCKET_PATH)\n' "$(safe_path "$CMUX_SOCKET_PATH")" | |
| elif [[ -n "${CMUX_SOCKET:-}" ]]; then | |
| printf 'cmux_socket_path: %s (from CMUX_SOCKET)\n' "$(safe_path "$CMUX_SOCKET")" | |
| else | |
| printf 'cmux_socket_path: unset\n' | |
| fi |
🤖 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 `@skills/cmux-diagnostics/scripts/cmux-diagnostics` around lines 104 - 109, The
conditionals that check CMUX_SOCKET_PATH and CMUX_SOCKET should use non-empty
tests so empty values don't mask fallbacks: replace the presence checks using
"${CMUX_SOCKET_PATH+x}" and "${CMUX_SOCKET+x}" with non-empty checks like -n
"${CMUX_SOCKET_PATH:-}" and -n "${CMUX_SOCKET:-}" in the conditional block that
prints cmux_socket_path (which uses safe_path to format the value), so an empty
CMUX_SOCKET_PATH will no longer prevent falling back to CMUX_SOCKET.
All actionable CodeRabbit findings from this review were fixed in follow-up commits; latest CodeRabbit check is passing/skipped.
This branch was successfully deployed
1 participant
Summary:
Testing:
Note
Low Risk
Low risk: adds new skill docs plus a read-only diagnostics script and updates the docs/skill catalog, with no changes to runtime app logic.
Overview
Adds two new end-user skills:
cmux-customization(guidance for safely editingcmux.jsonactions/layouts/shortcuts and deferring terminal prefs to Ghostty) andcmux-diagnostics(support-safe, read-only health checks plus a bundledcmux-diagnosticsscript with optional--include-context).Removes the
cmux-debug-windowsskill and snapshot script from this repo’s published skill set.Updates the Skills docs UI and EN/JA strings to list the new skills, drop Debug Windows/suggested-skill entries, and prefer installation via Vercel’s
npx skills add ...flow withskills.shas a fallback.Reviewed by Cursor Bugbot for commit b8be110. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds end-user skills
cmux-customizationandcmux-diagnostics, updates docs to prefer the Vercelskillsinstall flow, and removes the publiccmux-debug-windowsskill. The diagnostics report now redacts home paths, skips socket sentinels by default, and safely handles special files.New Features
cmux-customization: Schema-safe edits tocmux.json(actions, layouts, toolbar, Command Palette, shortcuts); defers terminal prefs to Ghostty; includes examples and reload/validate steps.cmux-diagnostics: Read-only checks for CLI/socket, hooks, session restore, settings, and agent binaries; addsskills/cmux-diagnostics/scripts/cmux-diagnosticswith optional--include-context; redacts$HOME, skips socket sentinels, and treats non-regular files safely.npx skills add manaflow-ai/cmux; shows both “Install with Vercelskills” and “Install withskills.sh”; removes Debug Windows and outdated suggestions.Migration
cmux-debug-windowsis no longer published here; use the version in cmuxterm-hq.skills:npx skills add manaflow-ai/cmux --all -g -y(or--skill cmux-diagnosticsas needed). Useskills.shonly for Codex’s directory.Written for commit b8be110. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Documentation
Removals