docs: fix install snippets across skills pages#27
Conversation
The previous pass introduced a broken install flow. The underlying issue: `degit webrenew/mogplex-docs/skills mogplex-skills` copies the *contents* of skills/ into mogplex-skills/, so there is no nested `skills/` directory — but the subsequent `cp -R skills/*` commands assumed there was. Unified pattern across both docs pages: ```bash # Pick one fetch option, which sets a SKILLS path variable: git clone https://github.com/webrenew/mogplex-docs.git && SKILLS=mogplex-docs/skills # or npx -y degit webrenew/mogplex-docs/skills mogplex-skills && SKILLS=mogplex-skills # Then copy: cp -R "$SKILLS"/* ~/.claude/skills/ ``` Applied to: - content/docs/extend/skills.mdx — replaces the broken `cd mogplex-skills && cd ..` and the following `cp -R skills/*` block - content/docs/cli/skills/index.mdx — replaces the inconsistent `cp -R .` + `cp -R /path/to/mogplex-skills/*` hybrid with the same unified pattern - skills/README.md — adds a "run from repo root" note and a second command for the degit case so readers following the in-repo README have a working command regardless of fetch method
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Mogplex PR Review
Status: Attention needed
The core fix is correct and well-reasoned: replacing the broken cd mogplex-skills && cd .. / cp -R skills/* pattern with a $SKILLS variable that accounts for degit's flat-copy behavior. All three files are consistent with each other. One minor inconsistency in skills/README.md is worth a quick fix before merging.
1 finding was added inline.
| mkdir -p .claude/skills && cp -R skills/* .claude/skills/ | ||
| ``` | ||
|
|
||
| If you pulled the skills tree with `npx degit webrenew/mogplex-docs/skills mogplex-skills` instead (so there is no outer `skills/` directory), run the copy as `cp -R mogplex-skills/* ~/.claude/skills/`. |
There was a problem hiding this comment.
Suggestion: skills/README.md degit note only covers the global install path
The new sentence added at the end of the install section reads:
If you pulled the skills tree with
npx degit … mogplex-skillsinstead (so there is no outerskills/directory), run the copy ascp -R mogplex-skills/* ~/.claude/skills/.
It only shows the global target (~/.claude/skills/). The block directly above it documents both global and per-project install, so a reader who wants per-project install via degit has to mentally substitute the path themselves.
Suggestion — expand the note to cover both variants, matching the structure above it:
If you pulled the skills tree with `npx degit webrenew/mogplex-docs/skills mogplex-skills` instead (so there is no outer `skills/` directory):
```bash
# Global
mkdir -p ~/.claude/skills && cp -R mogplex-skills/* ~/.claude/skills/
# Or per-project (run from your project root)
mkdir -p .claude/skills && cp -R mogplex-skills/* .claude/skills/
This keeps `README.md` fully self-contained for degit users, consistent with how `content/docs/extend/skills.mdx` and `content/docs/cli/skills/index.mdx` handle it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99af71a560
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Or per-project (run from your project root, not the downloaded skills dir) | ||
| mkdir -p .claude/skills && cp -R /path/to/mogplex-skills/* .claude/skills/ | ||
| # Or per-project (run from your project root) | ||
| mkdir -p .claude/skills && cp -R "$SKILLS"/* .claude/skills/ |
There was a problem hiding this comment.
Use absolute SKILLS path for per-project copy
The per-project command tells users to run from their project root, but SKILLS is set to a relative path (mogplex-docs/skills or mogplex-skills), so cp -R "$SKILLS"/* .claude/skills/ fails as soon as they cd to a different directory. This makes the documented per-project install path non-functional for the common case where the app repo is separate from the downloaded docs/skills folder (the same pattern appears in content/docs/extend/skills.mdx).
Useful? React with 👍 / 👎.
Follow-up to #26. Addresses all four install-snippet findings from the PR review on #26.
Root cause
The previous pass introduced
npx degit webrenew/mogplex-docs/skills mogplex-skillsas Option B without accounting for the fact that degit copies the contents ofskills/into the target directory — there is no nestedskills/folder afterward. The subsequentcp -R skills/*command therefore fails withNo such file or directory. On top of that, the two docs pages used different copy patterns, making the inconsistency confusing.Fix: one unified pattern across both pages
Let the reader set a
SKILLSpath variable from whichever fetch option they pick, then use a single copy block:Files changed
content/docs/extend/skills.mdx— replaces brokencd mogplex-skills && cd ..and the mismatchedcp -R skills/*block.content/docs/cli/skills/index.mdx— replaces the inconsistentcp -R .+cp -R /path/to/mogplex-skills/*hybrid with the unified\$SKILLSpattern.skills/README.md— adds "run from repo root" note and an explicitmogplex-skills/*command for the degit case so the in-repo README is accurate regardless of fetch method.Test plan